From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Oliver Neukum <oliver@neukum.org>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Paul Stoffregen <paul@pjrc.com>
Subject: [PATCH] USB: cdc-acm: remove unneeded spin_lock_irqsave/restore on write path
Date: Fri, 7 Jun 2013 11:32:47 -0700 [thread overview]
Message-ID: <20130607183247.GA6850@kroah.com> (raw)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 <paul@pjrc.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/usb/class/cdc-acm.c | 51 +++++++++++++++-----------------------------
1 file changed, 18 insertions(+), 33 deletions(-)
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -216,38 +216,6 @@ static int acm_start_wb(struct acm *acm,
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,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);
+ 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);
if (stat < 0)
return stat;
return count;
next reply other threads:[~2013-06-07 18:32 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-07 18:32 Greg Kroah-Hartman [this message]
2013-06-07 18:38 ` [PATCH] USB: cdc-acm: remove unneeded spin_lock_irqsave/restore on write path Sergei Shtylyov
2013-06-07 18:41 ` Greg Kroah-Hartman
2013-06-07 18:42 ` [PATCH v2] " Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130607183247.GA6850@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=oliver@neukum.org \
--cc=paul@pjrc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox