From owner-svn-src-all@FreeBSD.ORG Fri Aug 7 09:56:21 2009 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DFBF2106566B; Fri, 7 Aug 2009 09:56:21 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail08.syd.optusnet.com.au (mail08.syd.optusnet.com.au [211.29.132.189]) by mx1.freebsd.org (Postfix) with ESMTP id 5D8FC8FC15; Fri, 7 Aug 2009 09:56:21 +0000 (UTC) Received: from besplex.bde.org (c122-106-150-32.carlnfd1.nsw.optusnet.com.au [122.106.150.32]) by mail08.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id n779u9G4010353 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 7 Aug 2009 19:56:11 +1000 Date: Fri, 7 Aug 2009 19:56:10 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "M. Warner Losh" In-Reply-To: <20090807.005400.-1749708164.imp@bsdimp.com> Message-ID: <20090807181536.P848@besplex.bde.org> References: <20090804031407.GA8974@hub.freebsd.org> <200908070830.47894.hselasky@c2i.net> <20090807.005400.-1749708164.imp@bsdimp.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: src-committers@FreeBSD.org, nparhar@gmail.com, svn-src-all@FreeBSD.org, alfred@FreeBSD.org, hselasky@c2i.net, rwatson@FreeBSD.org, brde@optusnet.com.au, svn-src-head@FreeBSD.org Subject: Re: svn commit: r195960 - in head/sys/dev/usb: . controller input X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Aug 2009 09:56:22 -0000 On Fri, 7 Aug 2009, M. Warner Losh wrote: > In message: <200908070830.47894.hselasky@c2i.net> > Hans Petter Selasky 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 > : > : 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