USB Mass Storage Device

[Giovanni]

The only problem I can see are the missing chSysLockFromIsr() and chSysUnlockFromIsr() in the callback, that could trigger a race condition.

You could also use the S variant of the semaphore wait and move it within the lock, that would eliminate entirely the case where the semaphore is signaled first.

Giovanni

[Badger]

Thanks for the tips, I have tried that but I still have the problem. I don't believe it is a result of timing, but for whatever reason signalling the semaphore and then waiting on it doesn't immediately release the thread.

I think there are two states when the usb callback is called:

• The thread is waiting on the binary semaphore, so the state of the semaphore is taken and the effect of chBSemSignal() is to release the waiting thread but keep the semaphore in the taken state
• The thread is not yet waiting, it is busy doing other things. But the semaphore is still in the taken state (either from initialisation or from a completed transfer), so the callback calls chBSemSignal() which puts the semaphore in an untaken state. When the thread is finished working and calls chBSemWait() it finds that the semaphore is untaken so continues without waiting

Is my understanding correct? it is the later state that does not work.

I've put the code on pastebin, for some reason the forum doesn't like .c attachments!

[Giovanni]

Your understanding is correct, the problem is that I have no explanation yet

Can you try to step into the wait function ?

BTW, are we waiting/signaling the same semaphore?

Giovanni

[Badger]

I changed the code to always use the same statically declared structure containing the BSem to make sure it isn't using a dodgy pointer.

Using the debugger I can see that before it calls chBSemWait the count is already -1, which I think means that there is already another thread queued (which there shouldn't be!). So maybe I am waiting on the semaphore one too many times.. need to think about this.

[Giovanni]

Be careful about hidden race conditions, when you wait on the semaphore then interrupts are re-enabled even if you are within a lock/unlock because you switch to another thread that does an "unlock".

Things like:
1) lock
2) wait or signal or anything that can reschedule
3) unlock

is *not* atomic. In the past I hit two very bad bugs related to this kind of error.

Giovanni

[Badger]

Thanks for the tip Giovanni, but I'm not sure how I should avoid the race conditions?

I put a break point in the usb callback code:

Code: Select all

   chSysLockFromIsr();
   chBSemSignalI(&UMSD1.bsem);
   chSysUnlockFromIsr();

What I'm finding is that in the early cases in which the callback wakes the worker thread, bsem.cnt = -2. But when the block write takes longer than the usb transaction, bsem.cnt = -1 before chBSemSignalI(), and 0 after it signals the semaphore. But the thread that is waiting on it never wakes.

As far as I can tell there are two issues:

1) cnt drops down to -2, does this imply that I have two chBSemWait calls before the semaphore is signalled? if I'm only ever calling chBSemWait from one thread shouldn't it be impossible to get to this state?
2) even when cnt gets to 0 the thread does not wait.

[Giovanni]

Hi,

In that case there is no race condition because the "I" class functions don't reschedule, I should have specified this.

If the counter goes to -2 it means that there are two threads waiting, this cannot be right in your scenario, try to catch this condition.

Giovanni

[Badger]

Here's what happens:

• BSem initialised, cnt goes from 0 to 0
• worker thread calls chBSemWaitS(), cnt is at 0
• if I step into chBSemWaitS() then the function does not return (unless I send usb commands from host). This is the expected behaviour
• if I Step over chBSemWaitS() then after a few seconds it appears as if the debugger has moved out of chBSemWaitS() but cnt is still -1. The return value is not set

Code: Select all

   chSysLock();
   msg_t res = chBSemWaitS(&UMSD1.bsem);
   chSysUnlock();

I'm not very good at using a debugger, so I'm not sure if this is usual behaviour, but if I hover my mouse over the variable res in the above chunk of code it does not give me a value like it does if I hover over bsem.

From what I've been experiencing, it does look as if two threads are trying to wait on the semaphore. If I put a breakpoint on chBSemWaitS is there someway I can see which thread is currently active? I have the chibios eclipse plugin running and it appears to work, I just don't know how to use it

[Giovanni]

Unless you started two threads on that function this should not be possible. Could you post the whole code? this is really strange.

Giovanni

[Badger]

I solved it! I had some code in the usb event handler that was waking the worker thread once the usb enumeration was complete. Basically I Was calling chSchReadyI() on a thread that was waiting on a semaphore. Clearly this was not what I should be doing!

anyway, it appears to work now, so I'll tidy up the code and put it on github.

Thanks for the help