svn commit: r195960 - in head/sys/dev/usb: . controller input

Bruce Evans brde at optusnet.com.au
Fri Aug 7 09:56:22 UTC 2009


On Fri, 7 Aug 2009, M. Warner Losh wrote:

> In message: <200908070830.47894.hselasky at c2i.net>
>            Hans Petter Selasky <hselasky at c2i.net> writes:
> : On Thursday 06 August 2009 21:47:16 Navdeep Parhar wrote:
> : > ...
> : > Simple sequence of steps to reproduce problem:
> : > ctrl-alt-esc on the USB keyboard
> : > db> c<return>
> :
> : This is like expected.
> :
> : Once paniced, USB operation is blocked on the USB controller which the
> : keyboard belongs to, because USB does not receive any polling-complete call,
> : so that it can clean up the state in the USB controller! This mainly has to do
> : with avoid calling wakeup() during polling.
> :
> : To avoid wakeup() calls, USB sets some bits, which must be cleared when
> : polling is complete, which is currently not done, because USB doesn't know
> : when polling is complete ...
>
> Polling isn't supposed to work like this...  The rest of the system
> effects a poll without these side effects.

Yes, polling is complete when the call to the console driver i/o
function returns, although this is broken as designed.  It requires a
full (enough) reentrant (enough) context switch and on every call to
a console driver i/o function, with a full (enough) hardware initialization
on entry and a switch back to the old context on return.  But this is
hard to implement for recalcitrant and/or complicated hardware, and it
cannot work for the cn_checkc() i/o function (which is now usually
misspelled cn_getc()), since switching the context back on return gives
up control so other functions (typically interrupt handlers) may eat
the console input.  Only the sio console driver does it AFAIK.

A non-broken design would involve calling a console driver open, close
or ioctl function switch to and from console mode at suitable points.
For ddb, the suitable points are on entry and exit from interactive
mode, at most once each for every entry into ddb (not on every entry
point since that would thrash the i/o subsystem unnecessarily for
things like stepping to the next return instruction; unnecessary (and
necessary) thrashing is most obvious if you have a correctly implemented
video console driver which must switch the whole physical screen context
unless console output uses a physically separate screen (not supported
in FreeBSD).  For other uses, the suitable points are unclear.  Obviously,
single printfs should be atomic, so the output part of the console
should be switched to and from at most once per printf, and printf
could do this easily, but that is not enough for printing long messages
or even for short messages that are built up with multiple printfs.
However, nothing better seems practical, since it would be a large
burden for all code that wants to use a set of printfs to have to wrap
the set with console driver open/close calls.  Therefore, the open/close
calls belong in printf right next to my uncommitted serialization for
printfs.  Serialization is closely related to reserving the console
output device -- it would be a similarly too-large burden for all code
that wants to use printf to have to wrap the printfs with serialization
calls.

Full context switches for console output (of a single physical screen)
are only best for ddb, since for ordinary printfs you want to see the
output, so putting it in an inactive virtual console is not so good,
and putting it on a separate virtual console and switching to that is
also not so good (the difference for ddb is mainly that it is natural
to switch the console on entry to interactive mode so as to actually
work on it).  So non-ddb printf output should go to a virtual console
(or just a buffer) even if the console driver doesn't support virtual
consoles.  printf already has some support for this (buffering so as
to do the output later to ptys).

The cn_dbctl() console entry point was designed to fix part of this
problem, but it was only used by syscons and this use was removed after
removing the (needed :-() calls to it.  The problem is larger than I
noticed when I designed it -- I thought that there was only a problem
for ddb mode, and that ddb mode was handled better by requiring the
console driver to be reentrant (reentrancy is best it it is possible
and works -- the idea is to work in all states, and having open/close
functions to switch the state gives even more states).  However,
problems for non-ddb mode became obvious when the multiple consoles
support code was added -- it made the cn_getc() interface useless and
the cn_checkc() interface primary, since all active consoles must be
polled for input together, and problems for ddb mode would be obvious
if syscons correctly context-switched the screen.  cn_dbctl() should
have been named cn_ioctl() for general ioctls, and it could be abused
for open/close with less abuse than having separate open/close entry
points, since the needed open/close functions are more like ioctls
than userland open/close (since you already have descriptors and want
to modify the state of the device).

syscons's use of cn_dbctl was simply:
- cn_dbctl passes TRUE on entry and FALSE on exit; these can be counted
   so as to implement full reentrancy, but syscons counts them only so as
   to avoid state changes except on entries that are not reentries.
   (Reentries are not supported by ddb so they shouldn't actually occur.)
- on entry (not reentry), syscons just stops the screen timer, hides the
   mouse, unlocks vty switching, switches to vty0, forces an update of
   the physical screen
- on entry (including reentry), syscons increments its private variable
   `debugger'.
- `debugger' is used mainly to avoid abusing ddb's private variable
   db_active.  Various operations that are invalid in ddb mode (not nearly
   all, and mainly ones involving the screen timer and calling wakeup())
   are avoided when `debugger' is set.
Now, db_active is replaced by kdb_active and it is abused a lot in other
subsystems but not nearly enough here.  syscons still uses `debugger', but
`debugger' is never initialized (except statically to 0).

Bruce


More information about the svn-src-all mailing list