Date: Sat, 15 Mar 2014 10:26:26 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Rui Paulo <rpaulo@freebsd.org> Cc: Hans Petter Selasky <hps@bitfrost.no>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r262972 - head/sys/dev/usb/input Message-ID: <20140315075248.Y964@besplex.bde.org> In-Reply-To: <C486FB67-41F4-4E13-8B91-A3AD1203F139@FreeBSD.org> References: <201403100852.s2A8qUdC045704@svn.freebsd.org> <D62898EB-F179-416B-A481-68604873477C@FreeBSD.org> <5322CCC5.7020608@bitfrost.no> <C486FB67-41F4-4E13-8B91-A3AD1203F139@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 14 Mar 2014, Rui Paulo wrote: > On 14 Mar 2014, at 02:32, Hans Petter Selasky <hps@bitfrost.no> wrote: > >> On 03/14/14 03:15, Rui Paulo wrote: >>> On 10 Mar 2014, at 01:52, Hans Petter Selasky <hselasky@freebsd.org> wrote: [>* excessive quoting deleted] >>>> Modified: head/sys/dev/usb/input/ukbd.c >>>> ============================================================================== >>>> --- head/sys/dev/usb/input/ukbd.c Mon Mar 10 06:41:48 2014 (r262971) >>>> +++ head/sys/dev/usb/input/ukbd.c Mon Mar 10 08:52:30 2014 (r262972) >>>> @@ -1909,6 +1909,12 @@ ukbd_ioctl(keyboard_t *kbd, u_long cmd, >>>> int result; >>>> >>>> /* >>>> + * XXX Check of someone is calling us from a critical section: >>>> + */ >>>> + if (curthread->td_critnest != 0) >>>> + return (EDEADLK); >>> >>> Shouldn't this panic? >> >> This happens on shutdown, in some special case. Not sure if panic at shutdown is appropriate? > > I thought this was a programming error. Do we know the special cases and why it happens? Doesn't it already break setting of LEDs and perhaps other things in ddb (when ddb traps a in a critical section)? Panicing would give even better breakage. It is a programming error (or perhaps a hardware defect) to not allow setting LEDs in ddb. Some hardware makes this very difficult. VGA and AT keyboard hardware is simpler than USB hardware, but syscons gets this wrong, mostly in the opposite direction by allowing races. Serial hardware is simpler still, but some serial drivers still get this wrong. Working console drivers support single-stepping through their own (re)initialization (except parts of this hidden by the virtualization needed to make it work). Most reinitialization occurs in ioctls, where it is probably simpler than in resume. It includes things like LED handling: - someone hits caps lock. The keyboard interrupt handler calls code to set the LED for caps lock (if this is done in software) - someone is debugging keyboard code, perhaps even LED handling code, and execution stops in or before the LED handling code - the debugging includes keystrokes that should toggle LEDs. These may occur at any instruction in the LED handling code, where the reinitialization of the LEDs is in in an indeterminate state. The intermediate state should be locked, but debugger activity must not block on this. The keystrokes should have their normal action of toggling the LED, except possibly when it was in a half-way state -- then toggling it could go either way) - when the LED handling code finishes, or soon after, the LED state must not be indeterminate. It might be the opposite of what it would have been without any debugger activity, but only if this is consistent with the software state. Normal software state for things like caps lock has precedence over what the LED is showing. Any debugger activity that toggles caps lock should toggle the software state first and set the LED to match. Very occasionally, the states will get out of sync when an instruction modifies the hardware state. This should be fixed up as soon as possible. In syscons, related fixups occur automatically for VGA things since everything is refreshed from the software state often. LED toggling has to work in critical sections too. Then the LED code itself is not being reentered (unless parts of it are locked by critical sections), but there are other complications. Of course, the LED code must not use any interrupts or do any context switches in debugger mode. (This reminds me that the AT keyboard hardware and syscons software for it are horrible. It takes a complicated protocol and several milliseconds to toggle a LED. My first microcomputer in 1982 was much better. It took a single memory access and several microseconds to set a LED to an 8-bit color. Syscons has always used polled i/o to set LEDs. This blocks its interrupt handler (and with spltty() or Giant, too many other interrupt handlers) for too long. But it allows LED setting in debugger mode to work without additional complications or bugs.) Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140315075248.Y964>