From: Johan Hovold <jhovold@gmail.com>
To: Oliver Neukum <oneukum@suse.de>, Jiri Slaby <jslaby@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
Peter Hurley <peter@hurleysoftware.com>,
One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>,
Xiao Jin <jin.xiao@intel.com>,
david.a.cohen@linux.intel.com, yanmin.zhang@intel.com,
Johan Hovold <jhovold@gmail.com>
Subject: [RFC 2/2] USB: cdc-acm: fix broken runtime suspend
Date: Fri, 11 Apr 2014 11:41:25 +0200 [thread overview]
Message-ID: <1397209285-15471-2-git-send-email-jhovold@gmail.com> (raw)
In-Reply-To: <1397209285-15471-1-git-send-email-jhovold@gmail.com>
The current runtime suspend implementation is broken in several ways:
Firstly, it buffers only the first write request being made while
suspended -- any further writes are silently dropped.
Secondly, writes being dropped also leak write urbs which are never
reclaimed (until the device is unbound).
Thirdly, even the single buffered write is not cleared at shutdown
(which may happen before the device is resumed), something which can
lead to another urb leak.
Fix this by simple removing the broken and redundant buffer
implementation from the driver, and rely on the tty buffers instead.
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/usb/class/cdc-acm.c | 28 +++++++---------------------
drivers/usb/class/cdc-acm.h | 1 -
2 files changed, 7 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 900f7ff805ee..c85bf3062bba 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -646,12 +646,10 @@ static int acm_tty_write(struct tty_struct *tty,
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);
+ wb->use = 0;
+ usb_autopm_put_interface_async(acm->control);
spin_unlock_irqrestore(&acm->write_lock, flags);
- return count; /* A white lie */
+ return 0;
}
usb_mark_last_busy(acm->dev);
@@ -1540,7 +1538,6 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message)
static int acm_resume(struct usb_interface *intf)
{
struct acm *acm = usb_get_intfdata(intf);
- struct acm_wb *wb;
int rv = 0;
int cnt;
@@ -1554,25 +1551,14 @@ static int acm_resume(struct usb_interface *intf)
if (test_bit(ASYNCB_INITIALIZED, &acm->port.flags)) {
rv = usb_submit_urb(acm->ctrlurb, GFP_NOIO);
-
- spin_lock_irq(&acm->write_lock);
- if (acm->delayed_wb) {
- wb = acm->delayed_wb;
- acm->delayed_wb = NULL;
- spin_unlock_irq(&acm->write_lock);
- acm_start_wb(acm, wb);
- } else {
- spin_unlock_irq(&acm->write_lock);
- }
-
- /*
- * delayed error checking because we must
- * do the write path at all cost
- */
if (rv < 0)
goto err_out;
rv = acm_submit_read_urbs(acm, GFP_NOIO);
+ if (rv < 0)
+ goto err_out;
+
+ schedule_work(&acm->work);
}
err_out:
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index e38dc785808f..538bb1b3b0bc 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -120,7 +120,6 @@ struct acm {
unsigned int throttled:1; /* actually throttled */
unsigned int throttle_req:1; /* throttle requested */
u8 bInterval;
- struct acm_wb *delayed_wb; /* write queued for a device about to be woken */
};
#define CDC_DATA_INTERFACE_TYPE 0x0a
--
1.8.3.2
next prev parent reply other threads:[~2014-04-11 9:42 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-08 3:05 [PATCH] cdc-acm: some enhancement on acm delayed write Xiao Jin
2014-04-08 7:33 ` Johan Hovold
2014-04-08 10:22 ` Oliver Neukum
2014-04-11 9:45 ` Johan Hovold
2014-04-08 10:33 ` Oliver Neukum
2014-04-08 13:17 ` Johan Hovold
2014-04-08 13:38 ` Oliver Neukum
2014-04-08 13:52 ` Johan Hovold
2014-04-08 11:22 ` One Thousand Gnomes
2014-04-08 13:12 ` Johan Hovold
2014-04-09 14:57 ` Xiao Jin
2014-04-09 17:43 ` David Cohen
2014-04-10 8:02 ` Oliver Neukum
2014-04-10 22:51 ` Xiao Jin
2014-04-11 7:09 ` Oliver Neukum
2014-04-11 9:37 ` Johan Hovold
2014-04-11 9:41 ` [RFC 1/2] n_tty: fix dropped output characters Johan Hovold
2014-04-11 9:41 ` Johan Hovold [this message]
2014-04-14 12:53 ` One Thousand Gnomes
2014-04-14 13:05 ` Oliver Neukum
2014-04-14 14:04 ` One Thousand Gnomes
2014-04-14 13:27 ` Johan Hovold
2014-04-14 19:58 ` [PATCH] USB: cdc-acm: fix broken runtime suspend Johan Hovold
2014-04-15 8:24 ` Xiao Jin
2014-04-15 8:54 ` Johan Hovold
2014-04-15 8:35 ` Oliver Neukum
2014-04-15 9:13 ` Johan Hovold
2014-04-15 12:19 ` Johan Hovold
2014-05-24 14:42 ` Johan Hovold
2014-05-24 19:59 ` Greg Kroah-Hartman
2014-05-24 20:42 ` Johan Hovold
2014-05-26 17:22 ` [PATCH 00/63] USB: (mostly runtime PM) patches for v3.16-rc Johan Hovold
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=1397209285-15471-2-git-send-email-jhovold@gmail.com \
--to=jhovold@gmail.com \
--cc=david.a.cohen@linux.intel.com \
--cc=gnomes@lxorguk.ukuu.org.uk \
--cc=gregkh@linuxfoundation.org \
--cc=jin.xiao@intel.com \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=oneukum@suse.de \
--cc=peter@hurleysoftware.com \
--cc=yanmin.zhang@intel.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;
as well as URLs for NNTP newsgroup(s).