Date: Wed, 17 Sep 2008 10:52:15 -0700 From: "Maksim Yevmenkin" <maksim.yevmenkin@gmail.com> To: "Eygene Ryabinkin" <rea-fbsd@codelabs.ru> Cc: rik@freebsd.org, hackers@freebsd.org Subject: Re: kern/127446: [patch] fix race in sys/dev/kbdmux/kbdmux.c Message-ID: <bb4a86c70809171052t5e961675n139b2848e6addd0@mail.gmail.com> In-Reply-To: <gaH7cu1zt0Bw5e8Q2XfgrlKSRyY@iXA9ZWPrtc2I2BMzBXoToMd7YdQ> References: <20080917161633.9E2F717101@shadow.codelabs.ru> <bb4a86c70809170956x36234893he8894a49127b6b6e@mail.gmail.com> <gaH7cu1zt0Bw5e8Q2XfgrlKSRyY@iXA9ZWPrtc2I2BMzBXoToMd7YdQ>
next in thread | previous in thread | raw e-mail | index | archive | help
On 9/17/08, Eygene Ryabinkin <rea-fbsd@codelabs.ru> wrote: > Maxim, good day. > > Cc'ing this discuission to hackers@ -- I was just going to write > the separate letter on this topic to the list. > > Wed, Sep 17, 2008 at 09:56:14AM -0700, Maksim Yevmenkin wrote: > > have you tried to simply set KBDMUX_LOCK/UNLOCK() to > > mtx_lock/unlock(&Giant) ? > > Since kbdmux callout is initialized as non-MPSAFE, this will result in > double locking the Giant (at least I see it from the code). I am not > sure that this is very good -- had not yet verified that Giant is > recursive. yes, giant is recursive. i think it should be fine for now (and yes, i agree, its not very clean) > Can try it tomorrow. thanks > Since you had written the code and #if 0'ed the locking part, may I ask, > why? Are there any known issues or it was just not very good to > introduce locking at that time (rev. 1.1, 3 years ago)? because i did not want to touch every single keyboard driver, keyboard subsystem and syscons :) back then. since kbdmux is pretty much keyboard driver it was easier to leave it under giant locking. thanks, max
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bb4a86c70809171052t5e961675n139b2848e6addd0>