When writing this patch I was unsure of how to handle the left over data
in the buffer. The patch below just uses memmove to copy the data
remaining in the buffer to the base address of the buffer.
I decided that the simplicity of this was favourable to the complexity
of alternatives, such as adding a base offset value to the acm_rb
struct. However if it's felt that avoiding the memmove is desirable then
I'm happy to rework this patch to add an offset value to the acm_rb struct.
Regards,
Toby
On 21/03/2011 15:52, Toby Gray wrote:
When sending large quantities of data through a CDC ACM channel it is possible
for data to be lost when attempting to copy the data to the tty buffer. This
occurs due to the return value from tty_insert_flip_string not being checked.
This patch adds checking for how many bytes have been inserted into the tty
buffer and returns any remaining bytes back to the filled read buffer list.
Signed-off-by: Toby Gray<toby.gray@xxxxxxxxxxx>
---
drivers/usb/class/cdc-acm.c | 28 ++++++++++++++++++++++++----
1 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index f492a7f..63133c7 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -392,6 +392,7 @@ static void acm_rx_tasklet(unsigned long _acm)
struct acm_ru *rcv;
unsigned long flags;
unsigned char throttled;
+ int copied;
dbg("Entering acm_rx_tasklet");
@@ -423,12 +424,14 @@ next_buffer:
dbg("acm_rx_tasklet: procesing buf 0x%p, size = %d", buf, buf->size);
+ copied = 0;
if (tty) {
spin_lock_irqsave(&acm->throttle_lock, flags);
throttled = acm->throttle;
spin_unlock_irqrestore(&acm->throttle_lock, flags);
if (!throttled) {
- tty_insert_flip_string(tty, buf->base, buf->size);
+ copied = tty_insert_flip_string(tty,
+ buf->base, buf->size);
tty_flip_buffer_push(tty);
} else {
tty_kref_put(tty);
@@ -440,9 +443,26 @@ next_buffer:
}
}
- spin_lock_irqsave(&acm->read_lock, flags);
- list_add(&buf->list,&acm->spare_read_bufs);
- spin_unlock_irqrestore(&acm->read_lock, flags);
+ if (copied == buf->size || !tty) {
+ spin_lock_irqsave(&acm->read_lock, flags);
+ list_add(&buf->list,&acm->spare_read_bufs);
+ spin_unlock_irqrestore(&acm->read_lock, flags);
+ } else {
+ tty_kref_put(tty);
+ dbg("Partial buffer fill");
+ if (copied> 0) {
+ memmove(buf->base,
+ buf->base + copied,
+ buf->size - copied);
+ buf->size -= copied;
+ }
+
+ spin_lock_irqsave(&acm->read_lock, flags);
+ list_add(&buf->list,&acm->filled_read_bufs);
+ spin_unlock_irqrestore(&acm->read_lock, flags);
+ return;
+ }
+
goto next_buffer;
urbs:
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
