diff options
author | David Brownell <dbrownell@users.sourceforge.net> | 2008-08-06 21:44:12 -0400 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2008-08-13 20:32:57 -0400 |
commit | 934da4635c2d05cef474e5243ef05df95b2ad264 (patch) | |
tree | 0ce5d1645e8e6c79253e0b1133d65800122547e1 | |
parent | 672c4e1843c54227ff1bdf1fdd96f9c45c56aa85 (diff) |
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 <dbrownell@users.sourceforge.net>
Cc: David Engraf <david.engraf@netcom.eu>
Acked-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
-rw-r--r-- | drivers/usb/class/cdc-acm.c | 32 | ||||
-rw-r--r-- | drivers/usb/class/cdc-acm.h | 2 |
2 files changed, 3 insertions, 31 deletions
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 910247d191e8..a9dd28f446d8 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c | |||
@@ -144,11 +144,6 @@ static int acm_wb_is_avail(struct acm *acm) | |||
144 | return n; | 144 | return n; |
145 | } | 145 | } |
146 | 146 | ||
147 | static inline int acm_wb_is_used(struct acm *acm, int wbn) | ||
148 | { | ||
149 | return acm->wb[wbn].use; | ||
150 | } | ||
151 | |||
152 | /* | 147 | /* |
153 | * Finish write. | 148 | * Finish write. |
154 | */ | 149 | */ |
@@ -157,7 +152,6 @@ static void acm_write_done(struct acm *acm, struct acm_wb *wb) | |||
157 | unsigned long flags; | 152 | unsigned long flags; |
158 | 153 | ||
159 | spin_lock_irqsave(&acm->write_lock, flags); | 154 | spin_lock_irqsave(&acm->write_lock, flags); |
160 | acm->write_ready = 1; | ||
161 | wb->use = 0; | 155 | wb->use = 0; |
162 | acm->transmitting--; | 156 | acm->transmitting--; |
163 | spin_unlock_irqrestore(&acm->write_lock, flags); | 157 | spin_unlock_irqrestore(&acm->write_lock, flags); |
@@ -190,40 +184,25 @@ static int acm_start_wb(struct acm *acm, struct acm_wb *wb) | |||
190 | static int acm_write_start(struct acm *acm, int wbn) | 184 | static int acm_write_start(struct acm *acm, int wbn) |
191 | { | 185 | { |
192 | unsigned long flags; | 186 | unsigned long flags; |
193 | struct acm_wb *wb; | 187 | struct acm_wb *wb = &acm->wb[wbn]; |
194 | int rc; | 188 | int rc; |
195 | 189 | ||
196 | spin_lock_irqsave(&acm->write_lock, flags); | 190 | spin_lock_irqsave(&acm->write_lock, flags); |
197 | if (!acm->dev) { | 191 | if (!acm->dev) { |
192 | wb->use = 0; | ||
198 | spin_unlock_irqrestore(&acm->write_lock, flags); | 193 | spin_unlock_irqrestore(&acm->write_lock, flags); |
199 | return -ENODEV; | 194 | return -ENODEV; |
200 | } | 195 | } |
201 | 196 | ||
202 | if (!acm->write_ready) { | ||
203 | spin_unlock_irqrestore(&acm->write_lock, flags); | ||
204 | return 0; /* A white lie */ | ||
205 | } | ||
206 | |||
207 | wb = &acm->wb[wbn]; | ||
208 | if(acm_wb_is_avail(acm) <= 1) | ||
209 | acm->write_ready = 0; | ||
210 | |||
211 | dbg("%s susp_count: %d", __func__, acm->susp_count); | 197 | dbg("%s susp_count: %d", __func__, acm->susp_count); |
212 | if (acm->susp_count) { | 198 | if (acm->susp_count) { |
213 | acm->old_ready = acm->write_ready; | ||
214 | acm->delayed_wb = wb; | 199 | acm->delayed_wb = wb; |
215 | acm->write_ready = 0; | ||
216 | schedule_work(&acm->waker); | 200 | schedule_work(&acm->waker); |
217 | spin_unlock_irqrestore(&acm->write_lock, flags); | 201 | spin_unlock_irqrestore(&acm->write_lock, flags); |
218 | return 0; /* A white lie */ | 202 | return 0; /* A white lie */ |
219 | } | 203 | } |
220 | usb_mark_last_busy(acm->dev); | 204 | usb_mark_last_busy(acm->dev); |
221 | 205 | ||
222 | if (!acm_wb_is_used(acm, wbn)) { | ||
223 | spin_unlock_irqrestore(&acm->write_lock, flags); | ||
224 | return 0; | ||
225 | } | ||
226 | |||
227 | rc = acm_start_wb(acm, wb); | 206 | rc = acm_start_wb(acm, wb); |
228 | spin_unlock_irqrestore(&acm->write_lock, flags); | 207 | spin_unlock_irqrestore(&acm->write_lock, flags); |
229 | 208 | ||
@@ -512,7 +491,6 @@ static void acm_softint(struct work_struct *work) | |||
512 | static void acm_waker(struct work_struct *waker) | 491 | static void acm_waker(struct work_struct *waker) |
513 | { | 492 | { |
514 | struct acm *acm = container_of(waker, struct acm, waker); | 493 | struct acm *acm = container_of(waker, struct acm, waker); |
515 | unsigned long flags; | ||
516 | int rv; | 494 | int rv; |
517 | 495 | ||
518 | rv = usb_autopm_get_interface(acm->control); | 496 | rv = usb_autopm_get_interface(acm->control); |
@@ -524,9 +502,6 @@ static void acm_waker(struct work_struct *waker) | |||
524 | acm_start_wb(acm, acm->delayed_wb); | 502 | acm_start_wb(acm, acm->delayed_wb); |
525 | acm->delayed_wb = NULL; | 503 | acm->delayed_wb = NULL; |
526 | } | 504 | } |
527 | spin_lock_irqsave(&acm->write_lock, flags); | ||
528 | acm->write_ready = acm->old_ready; | ||
529 | spin_unlock_irqrestore(&acm->write_lock, flags); | ||
530 | usb_autopm_put_interface(acm->control); | 505 | usb_autopm_put_interface(acm->control); |
531 | } | 506 | } |
532 | 507 | ||
@@ -697,7 +672,7 @@ static int acm_tty_write_room(struct tty_struct *tty) | |||
697 | * Do not let the line discipline to know that we have a reserve, | 672 | * Do not let the line discipline to know that we have a reserve, |
698 | * or it might get too enthusiastic. | 673 | * or it might get too enthusiastic. |
699 | */ | 674 | */ |
700 | return (acm->write_ready && acm_wb_is_avail(acm)) ? acm->writesize : 0; | 675 | return acm_wb_is_avail(acm) ? acm->writesize : 0; |
701 | } | 676 | } |
702 | 677 | ||
703 | static int acm_tty_chars_in_buffer(struct tty_struct *tty) | 678 | static int acm_tty_chars_in_buffer(struct tty_struct *tty) |
@@ -1076,7 +1051,6 @@ skip_normal_probe: | |||
1076 | spin_lock_init(&acm->write_lock); | 1051 | spin_lock_init(&acm->write_lock); |
1077 | spin_lock_init(&acm->read_lock); | 1052 | spin_lock_init(&acm->read_lock); |
1078 | mutex_init(&acm->mutex); | 1053 | mutex_init(&acm->mutex); |
1079 | acm->write_ready = 1; | ||
1080 | acm->rx_endpoint = usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress); | 1054 | acm->rx_endpoint = usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress); |
1081 | 1055 | ||
1082 | buf = usb_buffer_alloc(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma); | 1056 | buf = usb_buffer_alloc(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma); |
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 { | |||
106 | struct list_head spare_read_bufs; | 106 | struct list_head spare_read_bufs; |
107 | struct list_head filled_read_bufs; | 107 | struct list_head filled_read_bufs; |
108 | int write_used; /* number of non-empty write buffers */ | 108 | int write_used; /* number of non-empty write buffers */ |
109 | int write_ready; /* write urb is not running */ | ||
110 | int old_ready; | ||
111 | int processing; | 109 | int processing; |
112 | int transmitting; | 110 | int transmitting; |
113 | spinlock_t write_lock; | 111 | spinlock_t write_lock; |