Re: CDC-ACM driver: data duplication due to throttling

On Sun, Feb 04, 2007 at 11:18:13PM +0100, Oliver Neukum wrote:
> Still, this would be untested, while delaying the buffers for later
> processing is tested.

Tested indeed, by me, and shown to deliver duplicate data.
But apart from that, I think you are right to push for the most
careful change.

> Could you redo it that way?

Ok. The version below is more similar in structure to the current code.
The essential change is that acm->throttle is now read only once so that
it can not change under our nose. Tested, still works.

Signed-off-by: Joris van Rantwijk <[email protected]> 


diff -upr -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-05 10:11:52.000000000 +0100
@@ -324,14 +324,20 @@ 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;
+	unsigned char throttled;
 	dbg("Entering acm_rx_tasklet");
 
-	if (!ACM_READY(acm) || acm->throttle)
+	if (!ACM_READY(acm))
+		return;
+
+	spin_lock(&acm->throttle_lock);
+	throttled = acm->throttle;
+	spin_unlock(&acm->throttle_lock);
+	if (throttled)
 		return;
 
 next_buffer:
 	spin_lock_irqsave(&acm->read_lock, flags);
 	if (list_empty(&acm->filled_read_bufs)) {
@@ -344,26 +350,24 @@ next_buffer:
 	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)
+	spin_lock(&acm->throttle_lock);
+	throttled = acm->throttle;
+	spin_unlock(&acm->throttle_lock);
+	if (!throttled)
 		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;
-		spin_unlock(&acm->throttle_lock);
+	if (throttled) {
+		dbg("Throttling noticed");
 		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;

-------------------------------------------------------------------------
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