usb_rndis: synchronize writes

The write function can be called concurrently by multiple threads. The
way it is implemented now means this desn't work, since there is no
guarantee the correct thread will be released from the semaphore by the
USB completion callback.

I tried to allow mutiple requests to run "in parallel" (really letting
the USB stack schedule them) by having he callback track which thread to
wake up (using send_message/receive_message as a synchronization tool)
but that still resulted in lockups.

The simplest solution is to ensure there is only a single thread doing a
write transaction at a time, which is achieved here with an extra mutex.

Fixes #18521.

Change-Id: I0b737acab6f5665cbe5b0e40a20ce99c16bdf21c
Reviewed-on: https://review.haiku-os.org/c/haiku/+/6707
Reviewed-by: Jérôme Duval <[email protected]>
This commit is contained in:
PulkoMandy
2023-07-24 14:47:23 +00:00
committed by Adrien Destugues
parent fef51dedd8
commit 45311bd6f9
2 changed files with 45 additions and 2 deletions
@@ -73,6 +73,7 @@ RNDISDevice::RNDISDevice(usb_device device)
fWriteEndpoint(0),
fNotifyReadSem(-1),
fNotifyWriteSem(-1),
fLockWriteSem(-1),
fNotifyControlSem(-1),
fReadHeader(NULL),
fLinkStateChangeSem(-1),
@@ -102,6 +103,12 @@ RNDISDevice::RNDISDevice(usb_device device)
return;
}
fLockWriteSem = create_sem(1, DRIVER_NAME"_lock_write");
if (fNotifyWriteSem < B_OK) {
TRACE_ALWAYS("failed to create write lock sem\n");
return;
}
fNotifyControlSem = create_sem(0, DRIVER_NAME"_notify_control");
if (fNotifyControlSem < B_OK) {
TRACE_ALWAYS("failed to create control notify sem\n");
@@ -123,6 +130,10 @@ RNDISDevice::~RNDISDevice()
delete_sem(fNotifyReadSem);
if (fNotifyWriteSem >= B_OK)
delete_sem(fNotifyWriteSem);
if (fLockWriteSem >= B_OK)
delete_sem(fLockWriteSem);
if (fNotifyControlSem >= B_OK)
delete_sem(fNotifyControlSem);
if (!fRemoved)
gUSBModule->cancel_queued_transfers(fNotifyEndpoint);
@@ -326,6 +337,26 @@ RNDISDevice::Read(uint8 *buffer, size_t *numBytes)
}
class SemLocker {
public:
SemLocker(sem_id sem)
: fSem(sem)
{
fStatus = acquire_sem(fSem);
}
~SemLocker()
{
if (fStatus == B_OK)
release_sem(fSem);
}
status_t fStatus;
private:
sem_id fSem;
};
status_t
RNDISDevice::Write(const uint8 *buffer, size_t *numBytes)
{
@@ -348,13 +379,23 @@ RNDISDevice::Write(const uint8 *buffer, size_t *numBytes)
vec[1].iov_base = (void*)buffer;
vec[1].iov_len = *numBytes;
status_t result = gUSBModule->queue_bulk_v(fWriteEndpoint, vec, 2, _WriteCallback, this);
SemLocker mutex(fLockWriteSem);
status_t result = mutex.fStatus;
if (result < B_OK) {
*numBytes = 0;
return result;
}
result = gUSBModule->queue_bulk_v(fWriteEndpoint, vec, 2, _WriteCallback, this);
if (result != B_OK) {
*numBytes = 0;
return result;
}
result = acquire_sem_etc(fNotifyWriteSem, 1, B_CAN_INTERRUPT, 0);
do {
result = acquire_sem_etc(fNotifyWriteSem, 1, B_CAN_INTERRUPT, 0);
} while (result == B_INTERRUPTED);
if (result < B_OK) {
*numBytes = 0;
return result;
@@ -372,6 +413,7 @@ RNDISDevice::Write(const uint8 *buffer, size_t *numBytes)
}
*numBytes = fActualLengthWrite;
return B_OK;
}
@@ -76,6 +76,7 @@ static void _NotifyCallback(void *cookie, int32 status,
int32 fStatusWrite;
sem_id fNotifyReadSem;
sem_id fNotifyWriteSem;
sem_id fLockWriteSem;
sem_id fNotifyControlSem;
uint8 fNotifyBuffer[8];