svn commit: r195960 - in head/sys/dev/usb: . controller input (regression patch)

[Bruce Evans]

On Mon, 3 Aug 2009, Robert Watson wrote:

> On Mon, 3 Aug 2009, Hans Petter Selasky wrote:
>> I think getmicrotime relies on interrupts, while microtime doesn't.
>> 
>> See "man microtime".
>
> You're right, but that doesn't make things better :-).  Some of the 
> tc_get_timecount() calls are safe in the DDB environment, but several are 
> not. In particular, tick_get_timecount_mp() and i8254_get_timecount() both 
> acquire locks, the former the thread scheduler lock, and the latter a 
> dedicated spinlock.  This produces the opportunity for rather nasty deadlocks 
> in DDB, especially tick_get_timecount_mp() on sparc64.

Hmm, why does tick_get_timecount_mp() need to bind curthread?  (I don't
understand sparc64.)

Any hardware clock potentially needs locking like that in
i8254_get_timecount(), but most hardware is not so bad.

DELAY() also wants to use the i8254 in some configurations.  On amd64
and i386, it uses a hack to avoid the lock in ddb mode.  Deadlock from
not doing this rarely occurred, and deadlock on the thread lock would
be more common.  tc_get_timecount() calls could do something similar
in some cases.  E.g., tick_get_timecount_mp() can just skip the locking
since entering ddb mode is stronger than sched binding, but
i8254_get_timecount() cannot do this safely so easily since it needs
to write to the hardware and the hardware has write-only state (the
state would have to be shadowed in memory).

Another problem with using microtime() is that the timecounter hardware
might wrap after a short time.  Again the i8254 timecounter is a problem.
At HZ=1000, it wraps after 1mS.  This can be handled by polling spinloops
if necessary by calling microtime() enough to detect all wraps and adding
up deltas of microtime()s to get the elapsed time.

Apart from deadlock, just calling mutex locking code from within ddb is
not supported (but it happens anywyay :-()).

> This was the bug I was actually looking for in your patch, but then misread 
> microtime() and concluded you had a different one. :-)  I would much rather 
> not have DDB rely on, for example, not contending thread_lock(), than have 
> key repeat in DDB.

If cngetc() worked right then you would also not have key repeat for polled
input outside of ddb :-), but polled input is probably used more in ddb
than anywhere else (e.g., for booting), so you would probably miss it
mainly in ddb.

Bruce