On Sat, Feb 03, 2007 at 04:18:24PM +0100, Oliver Neukum wrote:
> 1. Why not simply take the lock earlier?
And call the tty layer under acm->throttle_lock? I think not.
We could of course read acm->throttle once under the lock and
store its value in a local variable. I can write it like that if
you prefer it, but I actually think that my solution is simpler since
it never needs to push buffers back on filled_read_bufs.
Greetings,
Joris.
--
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 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.
Signed-off-by: Joris van Rantwijk <[EMAIL PROTECTED]>
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
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
