There seems to be a bug in the handling of device throttling in the
CDC-ACM driver. When the tty requests throttling, the same packet
is sometimes delivered to the tty and also pushed back on the list
of pending packets, resulting in delivery of duplicate data.
The problem is inside acm_rx_tasklet():
tty_buffer_request_room(tty, buf->size);
/*
* The next line is tricky: it reads acm->throttle without taking
* the corresponding lock.
*/
if (!acm->throttle)
tty_insert_flip_string(tty, buf->base, buf->size);
tty_flip_buffer_push(tty);
spin_lock(&acm->throttle_lock);
/*
* The next line reads acm->throttle again, but note that there
* is no guarantee that it holds the same value as a few moments
* ago. It is in fact likely that acm->throttle will have changed
* since throttle() is called from within tty_flip_buffer_push().
*/
if (acm->throttle) {
dbg("Throtteling noticed");
/*
* The next two lines are useless because i is always 0.
*/
memmove(buf->base, buf->base + i, buf->size - i);
buf->size -= i;
spin_unlock(&acm->throttle_lock);
spin_lock_irqsave(&acm->read_lock, flags);
/*
* The next line adds the buffer back to the pending list, while
* it may have been delivered to the tty already.
*/
list_add(&buf->list, &acm->filled_read_bufs);
spin_unlock_irqrestore(&acm->read_lock, flags);
return;
}
The proposed patch fixes the problem by unconditionally pushing the
packet to the tty and never pushing it back on the pending list.
It is reasonable to assume that the tty layer can store at least one
more packet after requesting throttling, since it does a fair amount
of internal buffering.
This patch was tested on i386, ohci-hcd with a custom USB device.
It appears to resolve the data duplication problem while still
correctly throttling the device.
Greetings,
Joris.
diff -urp -U5 linux-2.6.19.2-orig/drivers/usb/class/cdc-acm.c
linux-2.6.19.2/drivers/usb/class/cdc-acm.c
--- linux-2.6.19.2-orig/drivers/usb/class/cdc-acm.c 2007-01-10
20:10:37.000000000 +0100
+++ linux-2.6.19.2/drivers/usb/class/cdc-acm.c 2007-02-03 01:05:21.000000000
+0100
@@ -324,11 +324,10 @@ static void acm_rx_tasklet(unsigned long
struct acm *acm = (void *)_acm;
struct acm_rb *buf;
struct tty_struct *tty = acm->tty;
struct acm_ru *rcv;
unsigned long flags;
- int i = 0;
dbg("Entering acm_rx_tasklet");
if (!ACM_READY(acm) || acm->throttle)
return;
@@ -339,35 +338,27 @@ next_buffer:
goto urbs;
}
buf = list_entry(acm->filled_read_bufs.next,
struct acm_rb, list);
list_del(&buf->list);
+ list_add(&buf->list, &acm->spare_read_bufs);
spin_unlock_irqrestore(&acm->read_lock, flags);
dbg("acm_rx_tasklet: procesing buf 0x%p, size = %d", buf, buf->size);
tty_buffer_request_room(tty, buf->size);
- if (!acm->throttle)
- tty_insert_flip_string(tty, buf->base, buf->size);
+ tty_insert_flip_string(tty, buf->base, buf->size);
tty_flip_buffer_push(tty);
spin_lock(&acm->throttle_lock);
if (acm->throttle) {
- dbg("Throtteling noticed");
- memmove(buf->base, buf->base + i, buf->size - i);
- buf->size -= i;
+ dbg("Throttling noticed");
spin_unlock(&acm->throttle_lock);
- spin_lock_irqsave(&acm->read_lock, flags);
- list_add(&buf->list, &acm->filled_read_bufs);
- spin_unlock_irqrestore(&acm->read_lock, flags);
return;
}
spin_unlock(&acm->throttle_lock);
- spin_lock_irqsave(&acm->read_lock, flags);
- list_add(&buf->list, &acm->spare_read_bufs);
- spin_unlock_irqrestore(&acm->read_lock, flags);
goto next_buffer;
urbs:
while (!list_empty(&acm->spare_read_bufs)) {
spin_lock_irqsave(&acm->read_lock, flags);
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
