Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Nick Garnett <nickg at ecoscentric dot com>]

Bart Veer <bartv@ecoscentric.com> writes:

> >>>>> "Gary" == Gary Thomas <gary@mlbassoc.com> writes:
> 
>     Gary> On Thu, 2006-02-16 at 17:25 +0100, Andrew Lunn wrote:
>     >> On Thu, Feb 16, 2006 at 11:15:25AM -0500, Derek Bouius wrote:
>     >> 
>     >> > I am not registered for the mailing list, but peruse it once in a
>     >> > while, so I am not sure if my mail will go through to it. Feel free
>     >> > to repost it if it doesn't.
>     >> 
>     >> It got through.
>     >> 
>     >> > What we did to fix the locking issue was change the mutex to a
>     >> > semaphore. See the patch. It seems to work reliably.
>     >> 
>     >> Did you understand what the problem was with the condition variable?
>     >> Could you explain it?
> 
>     Gary> This seems like a pretty heavyweight solution.  I think that Bart
>     Gary> may want/need to review this before it's accepted into CVS.
> 
> Yes, although it has been a while since I looked at that code.
> 
> A problem with semaphores is that they are only available in kernel
> configurations. The driver API only has condition variables as a way
> of signalling events from a DSR to a thread. That is fine for
> complicated I/O, but in most of my drivers I only need something
> simple. A counting semaphore in the driver API would make it easier to
> write drivers, and would require less code and data than a
> mutex/condition variable combination.
> 
> Does anybody remember why counting semaphores were left out of the
> driver API?

In eCos device drivers are responsible for their own concurrency
control. While some drivers might be able to cope with two threads
executing in them concurrently, most cannot. They must therefore
implement some sort of mutual exclusion to serialize the threads. By
remarkable coincidence, a mutex does exactly the right thing.

Once a thread is in the driver it examines its state. Depending on
what it finds, it may need to wait for data to arrive, or start data
transmission, and then wait for it to finish. The wait takes the form
of a loop, testing some condition of the device driver and calling
cyg_dev_cond_wait().

Waiting on a condition variable unlocks the mutex, which may allow a
second thread through. This thread may want to do something else in
the device (transmit rather than receive, select, non-blocking IO,
ioctl etc.) and may either quickly exit or wait elsewhere. If it
follows the same path as the first thread it will end up queued behind
it on the same condition variable.

A semaphore just would not work in this situation. For a start it does
not atomically release the mutex, and just unlocking the mutex before
waiting on the semaphore (and relocking after) opens up a race
window. Also, the semaphore count is not very useful. The fact that
the ISR/DSR have run is encoded in the state of the device driver,
which is what the loop around the condition variable wait should be
testing. For example, in the serial drivers, threads wait for
characters to be inserted into the receive queue, or for space to
become available in the transmit queue. 

If the USB drivers are losing wakeups I can only assume that the wait
loops are testing the wrong conditions. It should not be too hard to
work out what the correct condition is and fix it.


-- 
Nick Garnett                                          eCos Kernel Architect
http://www.ecoscentric.com                     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss