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

[Bart Veer <bartv at ecoscentric dot com>]

>>>>> "Derek" == Derek Bouius <derek.bouius@sympatico.ca> writes:

    Derek> The problem is a race condition between checking the completed flag and executing the callback.
    Derek> I will describe the flow of the usbs_devtab_write:
    Derek> 1. Initialize the variables (signal, mutex, etc)
    Derek> 2. Setup the callback function - (Callback sets the signal) 
    Derek> 3. Start the transfer
    Derek> 4. Lock the mutex.
    Derek> 5. Check if wait.completed
    Derek> ...race condition....
    Derek> 	6. Wait for signal -- waits here forever in some cases if the callback was executed after #5 and before #6
    Derek> 7. Unlock the mutex.
    Derek> ...
    Derek> ..

    Derek> What happens in the lower level driver is that the (#3)
    Derek> start_tx_fn() enables the interrupt. That can fire
    Derek> immediately and call the callback from within the dsr.
    Derek> Executing #5 above can happen inbetween the enable of the
    Derek> interrupt and the actual interrupt.

    Derek> The same process can occur in the read.

I agree there is a race condition in the current code. My preferred
solution would be to replace the mutex/condition variable combo in
this code with a semaphore, as per other ecos-discuss postings. An
alternative approach for now, which works with the existing driver
API, is to add some scheduler locking as well. The patch below is
untested (I don't have any USB-capable hardware setup right now) but
should do the trick.

Bart

Index: usbs.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/usb/slave/current/src/usbs.c,v
retrieving revision 1.5
diff -u -r1.5 usbs.c
--- usbs.c	23 May 2002 23:06:36 -0000	1.5
+++ usbs.c	20 Feb 2006 13:37:06 -0000
@@ -111,9 +111,11 @@
     (*endpoint->start_tx_fn)(endpoint);
     
     cyg_drv_mutex_lock(&wait.lock);
+    cyg_drv_dsr_lock();
     while (!wait.completed) {
         cyg_drv_cond_wait(&wait.signal);
     }
+    cyg_drv_dsr_unlock();
     cyg_drv_mutex_unlock(&wait.lock);
     if (wait.result < 0) {
         result = wait.result;
@@ -155,9 +157,11 @@
     endpoint->complete_data     = (void*) &wait;
     (*endpoint->start_rx_fn)(endpoint);
     cyg_drv_mutex_lock(&wait.lock);
+    cyg_drv_dsr_lock();
     while (!wait.completed) {
         cyg_drv_cond_wait(&wait.signal);
     }
+    cyg_drv_dsr_unlock();
     cyg_drv_mutex_unlock(&wait.lock);
     if (wait.result < 0) {
         result = wait.result;


-- 
Bart Veer                       eCos Configuration 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