From: Havard Skinnemoen <hskinnemoen@google.com>
To: gregkh@suse.de
Cc: alan@linux.intel.com, davej@redhat.com, jslaby@suse.cz,
linux-kernel@vger.kernel.org,
Havard Skinnemoen <hskinnemoen@google.com>
Subject: [PATCH] cdc-acm: Kill ACM_READY() macro completely
Date: Fri, 9 Dec 2011 16:51:54 -0800 [thread overview]
Message-ID: <1323478314-29443-1-git-send-email-hskinnemoen@google.com> (raw)
In-Reply-To: <13234763473078@kroah.org>
The ACM_READY() macro doesn't seem to do anything useful, and it may
prevent tty_wait_until_sent() from working properly when called from
close.
Previously, acm_tty_chars_in_buffer() returned 0 whenever
acm->port.count was 0. This means close() could return before all the
data has actually been written.
Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
---
Hi Greg,
Thanks for picking up my patch. Unfortunately, it's not completely bug-free.
This fixes an issue I found while testing the patch: After writing something to
/dev/ttyACM0, the process would hang for a fairly long time (10-15 seconds)
when closing the device. This is because port.count drops to 0 before
tty_wait_until_sent_from_close() is called, which causes ACM_READY() to return
false, any outstanding writes never get reaped because the workqueue doesn't
get scheduled, tty_wait_until_sent never sees any progress and eventually times
out.
Please feel free to fold this into "cdc-acm: Fix potential deadlock (lockdep
warning)", or let me know if you want me to send you a v2 of that patch
instead.
drivers/usb/class/cdc-acm.c | 10 +---------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 3027ada..d9d9340 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -60,8 +60,6 @@ static struct acm *acm_table[ACM_TTY_MINORS];
static DEFINE_MUTEX(acm_table_lock);
-#define ACM_READY(acm) (acm && acm->dev && acm->port.count)
-
/*
* acm_table accessors
*/
@@ -319,9 +317,6 @@ static void acm_ctrl_irq(struct urb *urb)
goto exit;
}
- if (!ACM_READY(acm))
- goto exit;
-
usb_mark_last_busy(acm->dev);
data = (unsigned char *)(dr + 1);
@@ -481,8 +476,7 @@ static void acm_write_bulk(struct urb *urb)
spin_lock_irqsave(&acm->write_lock, flags);
acm_write_done(acm, wb);
spin_unlock_irqrestore(&acm->write_lock, flags);
- if (ACM_READY(acm))
- schedule_work(&acm->work);
+ schedule_work(&acm->work);
}
static void acm_softint(struct work_struct *work)
@@ -492,8 +486,6 @@ static void acm_softint(struct work_struct *work)
dev_vdbg(&acm->data->dev, "%s\n", __func__);
- if (!ACM_READY(acm))
- return;
tty = tty_port_tty_get(&acm->port);
if (!tty)
return;
--
1.7.3.1
next parent reply other threads:[~2011-12-10 0:52 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <13234763473078@kroah.org>
2011-12-10 0:51 ` Havard Skinnemoen [this message]
2011-12-10 3:32 ` Greg KH
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=1323478314-29443-1-git-send-email-hskinnemoen@google.com \
--to=hskinnemoen@google.com \
--cc=alan@linux.intel.com \
--cc=davej@redhat.com \
--cc=gregkh@suse.de \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
/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
all inboxes | Powered by JetHome®