USB: cdc-acm: remove unneeded spin_lock_irqsave/restore on write path

From: Greg Kroah-Hartman <[email protected]>

When writing data we were:
lock
do some work
unlock
call function
lock
do some work
unlock
return
return

It turns out, that "function" was only ever called in the one place, so instead of locking/unlocking for no good reason, just inline the function and only grab the lock once.

This has sped up the pathological case of sending 1 byte packets to a loop-back cdc-acm device from 49600 bytes per second to 50100 bytes a second on my workstation. A tiny increase yes, but noticable, and now the spinlock isn't the hottest thing on the perf graph anymore. Yes, we are still waiting for the hardware for the most part, but getting rid of a spin_lock_irqsave() call for every packet is still a good thing.

And we end up deleting lines of code, always a win overall.

This was found by using a Teensy 3.0 device and the test program and firmware located at:
http://www.pjrc.com/teensy/benchmark_usb_serial_receive.html

Reported-by Paul Stoffregen <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  drivers/usb/class/cdc-acm.c |   51 +++++++++++++++-----------------------------
  1 file changed, 18 insertions(+), 33 deletions(-)

drivers/usb/class/cdc-acm.c +0 -0

drivers/usb/class/cdc-acm.c +6 -0

@@ -653,13 +621,30 @@ static int acm_tty_write(struct tty_stru
  	}
  	wb = &acm->wb[wbn];
 
+	if (!acm->dev) {
+		wb->use = 0;
+		spin_unlock_irqrestore(&acm->write_lock, flags);
+		return -ENODEV;
+	}
+
  	count = (count > acm->writesize) ? acm->writesize : count;
  	dev_vdbg(&acm->data->dev, "%s - write %d\n", __func__, count);
  	memcpy(wb->buf, buf, count);
  	wb->len = count;

Hello.

Quoted 39 lines · Greg Kroah-Hartman

From: Greg Kroah-Hartman <[email protected]>

When writing data we were:
lock
do some work
unlock
call function
lock
do some work
unlock
return
return

It turns out, that "function" was only ever called in the one place, so instead of locking/unlocking for no good reason, just inline the function and only grab the lock once.

This has sped up the pathological case of sending 1 byte packets to a loop-back cdc-acm device from 49600 bytes per second to 50100 bytes a second on my workstation. A tiny increase yes, but noticable, and now the spinlock isn't the hottest thing on the perf graph anymore. Yes, we are still waiting for the hardware for the most part, but getting rid of a spin_lock_irqsave() call for every packet is still a good thing.

And we end up deleting lines of code, always a win overall.

This was found by using a Teensy 3.0 device and the test program and firmware located at:
http://www.pjrc.com/teensy/benchmark_usb_serial_receive.html

Reported-by Paul Stoffregen <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/usb/class/cdc-acm.c |   51 +++++++++++++++-----------------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

drivers/usb/class/cdc-acm.c +0 -0

[...]

Quoted 16 lines

drivers/usb/class/cdc-acm.c +7 -1

@@ -653,13 +621,30 @@ static int acm_tty_write(struct tty_stru
 	}
 	wb = &acm->wb[wbn];
+	if (!acm->dev) {
+		wb->use = 0;
+		spin_unlock_irqrestore(&acm->write_lock, flags);
+		return -ENODEV;
+	}
+
 	count = (count > acm->writesize) ? acm->writesize : count;
 	dev_vdbg(&acm->data->dev, "%s - write %d\n", __func__, count);
 	memcpy(wb->buf, buf, count);
 	wb->len = count;
 	spin_unlock_irqrestore(&acm->write_lock, flags);
-	stat = acm_write_start(acm, wbn);
+	usb_autopm_get_interface_async(acm->control);

drivers/usb/class/cdc-acm.c static int acm_tty_write(struct tty_stru · line 642

Quoted 6 lines

drivers/usb/class/cdc-acm.c +6 -0

+	if (acm->susp_count) {
+		if (!acm->delayed_wb)
+			acm->delayed_wb = wb;
+		else
+			usb_autopm_put_interface_async(acm->control);
+		spin_unlock_irqrestore(&acm->write_lock, flags);

But you've already dropped it above the *if* and not taken again?