USB: cdc-acm: remove unneeded spin_lock_irqsave/restore on write path
authorGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 7 Jun 2013 18:42:08 +0000 (11:42 -0700)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Mon, 17 Jun 2013 20:37:07 +0000 (13:37 -0700)
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 spinlock_irq_save() 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 <paul@pjrc.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/class/cdc-acm.c

index 9b1cbcf8fb7fcfbd0f3b576d207a90dd4421c62f..9f49bfe4c6f4a24e112a71ecabee13b6e59aec0a 100644 (file)
@@ -216,38 +216,6 @@ static int acm_start_wb(struct acm *acm, struct acm_wb *wb)
        return rc;
 }
 
-static int acm_write_start(struct acm *acm, int wbn)
-{
-       unsigned long flags;
-       struct acm_wb *wb = &acm->wb[wbn];
-       int rc;
-
-       spin_lock_irqsave(&acm->write_lock, flags);
-       if (!acm->dev) {
-               wb->use = 0;
-               spin_unlock_irqrestore(&acm->write_lock, flags);
-               return -ENODEV;
-       }
-
-       dev_vdbg(&acm->data->dev, "%s - susp_count %d\n", __func__,
-                                                       acm->susp_count);
-       usb_autopm_get_interface_async(acm->control);
-       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);
-               return 0;       /* A white lie */
-       }
-       usb_mark_last_busy(acm->dev);
-
-       rc = acm_start_wb(acm, wb);
-       spin_unlock_irqrestore(&acm->write_lock, flags);
-
-       return rc;
-
-}
 /*
  * attributes exported through sysfs
  */
@@ -653,13 +621,31 @@ static int acm_tty_write(struct tty_struct *tty,
        }
        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;
+
+       usb_autopm_get_interface_async(acm->control);
+       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);
+               return count;   /* A white lie */
+       }
+       usb_mark_last_busy(acm->dev);
+
+       stat = acm_start_wb(acm, wb);
        spin_unlock_irqrestore(&acm->write_lock, flags);
 
-       stat = acm_write_start(acm, wbn);
        if (stat < 0)
                return stat;
        return count;
This page took 0.035298 seconds and 5 git commands to generate.