Date: Thu, 18 Sep 2008 11:10:17 +0400 From: Eygene Ryabinkin <rea-fbsd@codelabs.ru> To: Maksim Yevmenkin <maksim.yevmenkin@gmail.com> Cc: rik@freebsd.org, hackers@freebsd.org, bug-followup@freebsd.org Subject: Re: kern/127446: [patch] fix race in sys/dev/kbdmux/kbdmux.c Message-ID: <OBTLESaczqaewZBU9qdf0FWZRHE@7/OI9n2WwGs0JPogavdPqCpU9P8> In-Reply-To: <bb4a86c70809171052t5e961675n139b2848e6addd0@mail.gmail.com> References: <20080917161633.9E2F717101@shadow.codelabs.ru> <bb4a86c70809170956x36234893he8894a49127b6b6e@mail.gmail.com> <gaH7cu1zt0Bw5e8Q2XfgrlKSRyY@iXA9ZWPrtc2I2BMzBXoToMd7YdQ> <bb4a86c70809171052t5e961675n139b2848e6addd0@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--RwxaKO075aXzzOz0
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Maksim, good day.
Wed, Sep 17, 2008 at 10:52:15AM -0700, Maksim Yevmenkin wrote:
> yes, giant is recursive. i think it should be fine for now (and yes, i
> agree, its not very clean)
OK, I had tried substituting KBDMUX_LOCK/UNLOCK with Giant operations --
it works as expected. I am sligtly concerned with the fact that, for
example, kbdmux_kbd_event() will grab Giant for some more time than the
initial version that protects only polling loop.
Probably it is not a big concern: the call chain from syscons's cngetc()
(via cncheckc(), syscons->cn_getc() =3D=3D sc_cngetc(), sccngetch(),
scgetc() and kbd_read_char()) to the kbdmux_read_char() is the only code
path that is not protected by Giant, being called from the kernel
directly:
- user-level code is notified about key presses by syscons that signals
tty layer about key press from sckbdevent();
- no other kbdmux routine seem to be called without being
Giant-protected (at least, I see no parts that can race with the
low-level keyboard events).
So the typical overhead of mangling with Giant at KBDMUX_{LOCK,UNLOCK}
is only in extra calls to the _mtx_lock_flags/_mtx_unlock_flags. The
only extra code that will hold Giant for a longer time is the kernel's
interactive input routines, but their performance is user-bounded ;))
There is one interesting question: I assume that clock interrupts are
lost when Giant is hold? If so, then holding Giant for some extra time
upon system's initialization when kernel waits for user input will
enable the system to drop bigger amounts of clock interrupts -- I assume
that the code in kbdmux_read_char() that translates the scancode takes
the biggest amount of run-time compared to the polling loop. Sure, the
overhead will be added just for the typed characters -- when there is no
input, overhead is rather small.
May be this will not lead to any bad (or visible/measurable)
consequences -- can't tell now.
--=20
Eygene
_ ___ _.--. #
\`.|\..----...-'` `-._.-'_.-'` # Remember that it is hard
/ ' ` , __.--' # to read the on-line manual =20
)/' _/ \ `-_, / # while single-stepping the kernel.
`-'" `"\_ ,_.-;_.-\_ ', fsc/as #
_.-'_./ {_.' ; / # -- FreeBSD Developers handbook=20
{_.-``-' {_/ #
--RwxaKO075aXzzOz0
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (FreeBSD)
iEYEARECAAYFAkjR/tkACgkQthUKNsbL7Yg0aQCfWt7wmcfpSO+b6MUYqatkYCLt
RjcAn24xyFKL23AE2lCIAQDV1ht0/Igi
=1kiS
-----END PGP SIGNATURE-----
--RwxaKO075aXzzOz0--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?OBTLESaczqaewZBU9qdf0FWZRHE>