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

One more thing to bother you with ;-)

Since acm->throttle_lock is only used to protect the simple flag byte
acm->throttle, it is somewhat more efficient and convenient to use
an atomic_t instead of a spinlock.

The included patch makes these changes. It applies on top of the most
recent version of the throttle fix. Feel free, of course, to drop this
one if you don't like to fix stuff when it ain't broken.

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


diff -urp -U5 linux-2.6.19.2-x/drivers/usb/class/cdc-acm.c linux-2.6.19.2/drivers/usb/class/cdc-acm.c
--- linux-2.6.19.2-x/drivers/usb/class/cdc-acm.c	2007-02-05 11:19:08.000000000 +0100
+++ linux-2.6.19.2/drivers/usb/class/cdc-acm.c	2007-02-05 11:04:35.000000000 +0100
@@ -65,10 +65,11 @@
 #include <linux/usb.h>
 #include <linux/usb/cdc.h>
 #include <asm/byteorder.h>
 #include <asm/unaligned.h>
 #include <linux/list.h>
+#include <asm/atomic.h>
 
 #include "cdc-acm.h"
 
 /*
  * Version Information
@@ -324,20 +325,14 @@ 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;
-	unsigned char throttled;
+	int throttled;
 	dbg("Entering acm_rx_tasklet");
 
-	if (!ACM_READY(acm))
-		return;
-
-	spin_lock(&acm->throttle_lock);
-	throttled = acm->throttle;
-	spin_unlock(&acm->throttle_lock);
-	if (throttled)
+	if (!ACM_READY(acm) || atomic_read(&acm->throttle))
 		return;
 
 next_buffer:
 	spin_lock_irqsave(&acm->read_lock, flags);
 	if (list_empty(&acm->filled_read_bufs)) {
@@ -350,13 +345,11 @@ 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);
-	spin_lock(&acm->throttle_lock);
-	throttled = acm->throttle;
-	spin_unlock(&acm->throttle_lock);
+	throttled = atomic_read(&acm->throttle);
 	if (!throttled)
 		tty_insert_flip_string(tty, buf->base, buf->size);
 	tty_flip_buffer_push(tty);
 
 	if (throttled) {
@@ -483,11 +476,11 @@ static int acm_tty_open(struct tty_struc
 	}
 	for (i = 0; i < acm->rx_buflimit; i++) {
 		list_add(&(acm->rb[i].list), &acm->spare_read_bufs);
 	}
 
-	acm->throttle = 0;
+	atomic_set(&acm->throttle, 0);
 
 	tasklet_schedule(&acm->urb_task);
 
 done:
 err_out:
@@ -600,23 +593,19 @@ static int acm_tty_chars_in_buffer(struc
 static void acm_tty_throttle(struct tty_struct *tty)
 {
 	struct acm *acm = tty->driver_data;
 	if (!ACM_READY(acm))
 		return;
-	spin_lock_bh(&acm->throttle_lock);
-	acm->throttle = 1;
-	spin_unlock_bh(&acm->throttle_lock);
+	atomic_set(&acm->throttle, 1);
 }
 
 static void acm_tty_unthrottle(struct tty_struct *tty)
 {
 	struct acm *acm = tty->driver_data;
 	if (!ACM_READY(acm))
 		return;
-	spin_lock_bh(&acm->throttle_lock);
-	acm->throttle = 0;
-	spin_unlock_bh(&acm->throttle_lock);
+	atomic_set(&acm->throttle, 0);
 	tasklet_schedule(&acm->urb_task);
 }
 
 static void acm_tty_break_ctl(struct tty_struct *tty, int state)
 {
@@ -933,11 +922,11 @@ skip_normal_probe:
 	acm->readsize = readsize;
 	acm->rx_buflimit = num_rx_buf;
 	acm->urb_task.func = acm_rx_tasklet;
 	acm->urb_task.data = (unsigned long) acm;
 	INIT_WORK(&acm->work, acm_softint, acm);
-	spin_lock_init(&acm->throttle_lock);
+	atomic_set(&acm->throttle, 0);
 	spin_lock_init(&acm->write_lock);
 	spin_lock_init(&acm->read_lock);
 	acm->write_ready = 1;
 	acm->rx_endpoint = usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress);
 
diff -urp -U5 linux-2.6.19.2-x/drivers/usb/class/cdc-acm.h linux-2.6.19.2/drivers/usb/class/cdc-acm.h
--- linux-2.6.19.2-x/drivers/usb/class/cdc-acm.h	2007-02-05 11:18:41.000000000 +0100
+++ linux-2.6.19.2/drivers/usb/class/cdc-acm.h	2007-02-05 10:56:12.000000000 +0100
@@ -108,18 +108,17 @@ struct acm {
 	int write_ready;				/* write urb is not running */
 	spinlock_t write_lock;
 	struct usb_cdc_line_coding line;		/* bits, stop, parity */
 	struct work_struct work;			/* work queue entry for line discipline waking up */
 	struct tasklet_struct urb_task;                 /* rx processing */
-	spinlock_t throttle_lock;			/* synchronize throtteling and read callback */
 	unsigned int ctrlin;				/* input control lines (DCD, DSR, RI, break, overruns) */
 	unsigned int ctrlout;				/* output control lines (DTR, RTS) */
 	unsigned int writesize;				/* max packet size for the output bulk endpoint */
 	unsigned int readsize,ctrlsize;			/* buffer sizes for freeing */
 	unsigned int used;				/* someone has this acm's device open */
 	unsigned int minor;				/* acm minor number */
-	unsigned char throttle;				/* throttled by tty layer */
+	atomic_t throttle;				/* throttled by tty layer */
 	unsigned char clocal;				/* termios CLOCAL */
 	unsigned int ctrl_caps;				/* control capabilities from the class specific header */
 };
 
 #define CDC_DATA_INTERFACE_TYPE	0x0a

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