This is the mail archive of the
ecos-discuss@sourceware.org
mailing list for the eCos project.
Re: How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...
Bart Veer <bartv@ecoscentric.com> writes:
> >>>>> "Nick" == Nick Garnett <nickg@ecoscentric.com> writes:
>
> <snip>
> >> Does anybody remember why counting semaphores were left out of the
> >> driver API?
>
> Nick> In eCos device drivers are responsible for their own
> Nick> concurrency control. While some drivers might be able to
> Nick> cope with two threads executing in them concurrently, most
> Nick> cannot. They must therefore implement some sort of mutual
> Nick> exclusion to serialize the threads. By remarkable
> Nick> coincidence, a mutex does exactly the right thing.
>
> Nick> Once a thread is in the driver it examines its state.
> Nick> Depending on what it finds, it may need to wait for data to
> Nick> arrive, or start data transmission, and then wait for it to
> Nick> finish. The wait takes the form of a loop, testing some
> Nick> condition of the device driver and calling
> Nick> cyg_dev_cond_wait().
>
> Nick> Waiting on a condition variable unlocks the mutex, which may
> Nick> allow a second thread through. This thread may want to do
> Nick> something else in the device (transmit rather than receive,
> Nick> select, non-blocking IO, ioctl etc.) and may either quickly
> Nick> exit or wait elsewhere. If it follows the same path as the
> Nick> first thread it will end up queued behind it on the same
> Nick> condition variable.
>
> Nick> A semaphore just would not work in this situation. For a
> Nick> start it does not atomically release the mutex, and just
> Nick> unlocking the mutex before waiting on the semaphore (and
> Nick> relocking after) opens up a race window. Also, the semaphore
> Nick> count is not very useful. The fact that the ISR/DSR have run
> Nick> is encoded in the state of the device driver, which is what
> Nick> the loop around the condition variable wait should be
> Nick> testing. For example, in the serial drivers, threads wait
> Nick> for characters to be inserted into the receive queue, or for
> Nick> space to become available in the transmit queue.
>
> Nick> If the USB drivers are losing wakeups I can only assume that
> Nick> the wait loops are testing the wrong conditions. It should
> Nick> not be too hard to work out what the correct condition is
> Nick> and fix it.
>
> OK, I think I understand your reasoning. It makes a great deal of
> sense in the context of serial drivers where e.g. data may arrive
> asynchronously and end up in ring buffers. However I am not sure it
> makes as much sense for other kinds of devices where no I/O happens
> until it is explicitly initiated. USB, I2C and SPI all fail into that
> category.
Synchronous devices are simply a degenerate case of the general case
of asynchronous devices. A mechanism that is suitable for asynchronous
devices will work for synchronous devices just as easily.
>
> To get things working right with condition variables the code would
> have to do something like the following:
>
> 1) in the device structure:
>
> cyg_drv_mutex_t lock;
> cyg_drv_cond_t wait;
> cyg_bool busy;
> cyg_bool completed;
>
> 2) at the start of an operation, typically somewhere in the
> device-specific I/O layer:
>
> cyg_drv_mutex_lock(&lock);
> while (busy) {
> cyg_drv_cond_wait(&wait);
> }
> busy = 1;
> (call into the device driver to perform the actual I/O)
> busy = 0;
> cyg_drv_mutex_unlock(&lock);
> cyg_drv_cond_signal(&wait);
>
> 3) inside the device driver, called with lock claimed:
>
> completed = 0;
> (start the I/O operation)
> cyg_drv_dsr_lock();
> while (!completed) {
> cyg_drv_cond_wait(&wait);
> }
> cyg_drv_dsr_unlock();
> (perform any clean-ups that may be necessary)
>
> 4) in the DSR:
>
> completed = 1;
> cyg_drv_cond_broadcast(&wait);
This corresponds almost exactly to what I would expect to be done, and
have implemented in the past. I would use a separate condition
variable to implement the gate-keeper to keep response time on the
event condition variable low.
Also, the completed variable is seldom needed. Instead the thread will
be testing whatever changes the ISR/DSR have made to the state of the
device. This is particularly important when there might be threads
waiting for a variety of different conditions to become true.
>
> What I would prefer to see is:
>
> 1) in the device structure:
>
> cyg_drv_mutex_t lock;
> cyg_drv_sem_t wait;
>
> 2) at the start of an I/O operation:
>
> cyg_drv_mutex_lock(&lock);
> (call into the device driver to perform the actual I/O)
> cyg_drv_mutex_unlock(&lock);
>
> 3) inside the device driver:
>
> (start the I/O operation)
> cyg_drv_sem_wait(&wait);
> (perform any clean-ups that may be necessary);
>
> 4) in the DSR:
>
> cyg_drv_sem_post(&wait);
>
>
> A semaphore and a condition variable are the same size (assuming
> 32-bit pointers), so the second approach saves space for the busy and
> completed flags. That may be two ints, two bytes, or just two bits in
> a flags field (although testing bits is likely to impose a code size
> penalty). It is still an overhead.
This is not actually true. a condition variable is just a thread
queue. A semaphore is a thread queue plus a counter. The counter is
effectively the equivalent of the completed variable above.
>
> Re. code size, the second approach eliminates four function calls,
> four assignments, and two loops. A fairly clear gain. If there are
> multiple threads attempting to perform I/O to the same device then
> the cyg_drv_cond_broadcast() call will also result in threads waking
> up and having to go to sleep again immediately because the device is
> still busy. This could be eliminated using a separate condition
> variable, but that adds other overhead.
The difference is less than you think. The completed variable is
logically unnecessary, the semaphore has the additional
counter. Internally semaphore wait is logically equvalent to
while( counter <= 0 )
cyg_drv_cond_wait( sem );
counter--;
It just moves the loop elsewhere.
There is always a tradeoff to be made between code size, data size,
runtime performance, real time response and maintainability.
>
> In terms of real-time behaviour, the condition variable approach has
> to lock the scheduler albeit for only a short time.
This will always be necessary, if the thread is to examine and
manipulate data shared with the ISR/DSR it is going to have to either
disable interrupts or lock the scheduler.
> More worryingly
> mutex priority inversion protection is not going to help. If you have
> a low priority thread currently performing I/O, a high priority thread
> wanting access to the same device, and an intermediate thread hogging
> the cpu, then when the I/O has completed you have a priority inversion
> situation: the high priority thread is blocked on a condition
> variable, not a semaphore.
Mutexes should not be used to queue threads for access to critical
sections in which the current owner suspends using something other
than a condition variable. Mutexes are for mutual exclusion, and
should be treated more like spinlocks. Long-term waiters should be
transferred to condition variables, and not left on the mutex.
>From a real time point of view, IO should be performed at the priority
of the requesting thread.
>
> So although I accept that there are situations where general purpose
> condition variables will be more appropriate, I believe the semaphore
> approach is better for many types of device. Hence we should
> reconsider adding semaphores to the driver API.
The API was designed to provide a template for how drivers should be
written. Providing too many options will result in drivers being badly
constructed, which may fail under unexpected conditions.
--
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