From 934da4635c2d05cef474e5243ef05df95b2ad264 Mon Sep 17 00:00:00 2001 From: David Brownell Date: Wed, 6 Aug 2008 18:44:12 -0700 Subject: usb: cdc-acm: stop dropping tx buffers The "increase cdc-acm write throughput" patch left in place two now-obsolete mechanisms, either of which can make the cdc-acm driver drop TX data (nasty!). This patch removes them: - The write_ready flag ... if an URB and buffer were found, they can (and should!) always be used. - TX path acm_wb_is_used() ... used when the buffer was just allocated, so that check is pointless. Also fix a won't-yet-matter leak of a write buffer on a disconnect path. Signed-off-by: David Brownell Cc: David Engraf Acked-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/usb/class/cdc-acm.h') diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h index 85c3aaaab7c5..94266362ca68 100644 --- a/drivers/usb/class/cdc-acm.h +++ b/drivers/usb/class/cdc-acm.h @@ -106,8 +106,6 @@ struct acm { struct list_head spare_read_bufs; struct list_head filled_read_bufs; int write_used; /* number of non-empty write buffers */ - int write_ready; /* write urb is not running */ - int old_ready; int processing; int transmitting; spinlock_t write_lock; -- cgit v1.2.3 From e5fbab51b4219fbd1dab28666affe38a920b5f7e Mon Sep 17 00:00:00 2001 From: David Brownell Date: Wed, 6 Aug 2008 18:46:10 -0700 Subject: usb: cdc-acm: drain writes on close Add a mechanism to let the write queue drain naturally before closing the TTY, rather than always losing that data. There is a timeout, so it can't wait too long. Provide missing locking inside acm_wb_is_avail(); it matters more now. Note, this presumes an earlier patch was applied, removing a call to this routine where the lock was held. Slightly improved diagnostics on write URB completion, so we can tell when a write URB gets killed and, if so, how much data it wrote first ... and so that I/O path is normally silent (and can't much change timings). Signed-off-by: David Brownell Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 39 ++++++++++++++++++++++++++++++++++----- drivers/usb/class/cdc-acm.h | 1 + 2 files changed, 35 insertions(+), 5 deletions(-) (limited to 'drivers/usb/class/cdc-acm.h') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index a9dd28f446d8..efc4373ededb 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -51,6 +51,7 @@ */ #undef DEBUG +#undef VERBOSE_DEBUG #include #include @@ -70,6 +71,9 @@ #include "cdc-acm.h" + +#define ACM_CLOSE_TIMEOUT 15 /* seconds to let writes drain */ + /* * Version Information */ @@ -85,6 +89,12 @@ static DEFINE_MUTEX(open_mutex); #define ACM_READY(acm) (acm && acm->dev && acm->used) +#ifdef VERBOSE_DEBUG +#define verbose 1 +#else +#define verbose 0 +#endif + /* * Functions for ACM control messages. */ @@ -136,11 +146,14 @@ static int acm_wb_alloc(struct acm *acm) static int acm_wb_is_avail(struct acm *acm) { int i, n; + unsigned long flags; n = ACM_NW; + spin_lock_irqsave(&acm->write_lock, flags); for (i = 0; i < ACM_NW; i++) { n -= acm->wb[i].use; } + spin_unlock_irqrestore(&acm->write_lock, flags); return n; } @@ -467,22 +480,28 @@ urbs: /* data interface wrote those outgoing bytes */ static void acm_write_bulk(struct urb *urb) { - struct acm *acm; struct acm_wb *wb = urb->context; + struct acm *acm = wb->instance; - dbg("Entering acm_write_bulk with status %d", urb->status); + if (verbose || urb->status + || (urb->actual_length != urb->transfer_buffer_length)) + dev_dbg(&acm->data->dev, "tx %d/%d bytes -- > %d\n", + urb->actual_length, + urb->transfer_buffer_length, + urb->status); - acm = wb->instance; acm_write_done(acm, wb); if (ACM_READY(acm)) schedule_work(&acm->work); + else + wake_up_interruptible(&acm->drain_wait); } static void acm_softint(struct work_struct *work) { struct acm *acm = container_of(work, struct acm, work); - dbg("Entering acm_softint."); - + + dev_vdbg(&acm->data->dev, "tx work\n"); if (!ACM_READY(acm)) return; tty_wakeup(acm->tty); @@ -603,6 +622,8 @@ static void acm_tty_unregister(struct acm *acm) kfree(acm); } +static int acm_tty_chars_in_buffer(struct tty_struct *tty); + static void acm_tty_close(struct tty_struct *tty, struct file *filp) { struct acm *acm = tty->driver_data; @@ -617,6 +638,13 @@ static void acm_tty_close(struct tty_struct *tty, struct file *filp) if (acm->dev) { usb_autopm_get_interface(acm->control); acm_set_control(acm, acm->ctrlout = 0); + + /* try letting the last writes drain naturally */ + wait_event_interruptible_timeout(acm->drain_wait, + (ACM_NW == acm_wb_is_avail(acm)) + || !acm->dev, + ACM_CLOSE_TIMEOUT * HZ); + usb_kill_urb(acm->ctrlurb); for (i = 0; i < ACM_NW; i++) usb_kill_urb(acm->wb[i].urb); @@ -1047,6 +1075,7 @@ skip_normal_probe: acm->urb_task.data = (unsigned long) acm; INIT_WORK(&acm->work, acm_softint); INIT_WORK(&acm->waker, acm_waker); + init_waitqueue_head(&acm->drain_wait); spin_lock_init(&acm->throttle_lock); spin_lock_init(&acm->write_lock); spin_lock_init(&acm->read_lock); diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h index 94266362ca68..1f95e7aa1b66 100644 --- a/drivers/usb/class/cdc-acm.h +++ b/drivers/usb/class/cdc-acm.h @@ -113,6 +113,7 @@ struct acm { struct usb_cdc_line_coding line; /* bits, stop, parity */ struct work_struct work; /* work queue entry for line discipline waking up */ struct work_struct waker; + wait_queue_head_t drain_wait; /* close processing */ struct tasklet_struct urb_task; /* rx processing */ spinlock_t throttle_lock; /* synchronize throtteling and read callback */ unsigned int ctrlin; /* input control lines (DCD, DSR, RI, break, overruns) */ -- cgit v1.2.3