Skip to content

Commit

Permalink
cdc-wdm: Clear read pipeline in case of error
Browse files Browse the repository at this point in the history
Implemented queued response handling. This queue is processed every time the
WDM_READ flag is cleared.

In case of a read error, userspace may not actually read the data, since the
driver returns an error through wdm_poll. After this, the underlying device may
attempt to send us more data, but the queue is not processed. While userspace is
also blocked, because the read error is never cleared.

After this patch, we proactively process the queue on a read error. If there was
an outstanding response to handle, that will clear the error (or go through the
same logic again, if another read error occurs). If there was no outstanding
response, this will bring the queue size back to 0, unblocking a future response
from the underlying device.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Tested-by: Robert Foss <robert.foss@collabora.com>
Acked-by: Oliver Neukum <oneukum@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Robert Foss authored and Greg Kroah-Hartman committed Aug 10, 2016
1 parent 0775a9c commit c1da59d
Showing 1 changed file with 29 additions and 10 deletions.
39 changes: 29 additions & 10 deletions drivers/usb/class/cdc-wdm.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ static void wdm_out_callback(struct urb *urb)
wake_up(&desc->wait);
}

/* forward declaration */
static int service_outstanding_interrupt(struct wdm_device *desc);

static void wdm_in_callback(struct urb *urb)
{
struct wdm_device *desc = urb->context;
Expand Down Expand Up @@ -189,7 +192,13 @@ static void wdm_in_callback(struct urb *urb)
}
}

desc->rerr = status;
/*
* only set a new error if there is no previous error.
* Errors are only cleared during read/open
*/
if (desc->rerr == 0)
desc->rerr = status;

if (length + desc->length > desc->wMaxCommand) {
/* The buffer would overflow */
set_bit(WDM_OVERFLOW, &desc->flags);
Expand Down Expand Up @@ -221,9 +230,19 @@ static void wdm_in_callback(struct urb *urb)
}

skip_error:
set_bit(WDM_READ, &desc->flags);
wake_up(&desc->wait);

set_bit(WDM_READ, &desc->flags);
if (desc->rerr) {
/*
* Since there was an error, userspace may decide to not read
* any data after poll'ing.
* We should respond to further attempts from the device to send
* data, so that we can get unstuck.
*/
service_outstanding_interrupt(desc);
}

unlock:
spin_unlock(&desc->iuspin);
}
Expand Down Expand Up @@ -457,17 +476,14 @@ static ssize_t wdm_write
}

/*
* clear WDM_READ flag and possibly submit the read urb if resp_count
* is non-zero.
* Submit the read urb if resp_count is non-zero.
*
* Called with desc->iuspin locked
*/
static int clear_wdm_read_flag(struct wdm_device *desc)
static int service_outstanding_interrupt(struct wdm_device *desc)
{
int rv = 0;

clear_bit(WDM_READ, &desc->flags);

/* submit read urb only if the device is waiting for it */
if (!desc->resp_count || !--desc->resp_count)
goto out;
Expand Down Expand Up @@ -559,7 +575,8 @@ static ssize_t wdm_read

if (!desc->reslength) { /* zero length read */
dev_dbg(&desc->intf->dev, "%s: zero length - clearing WDM_READ\n", __func__);
rv = clear_wdm_read_flag(desc);
clear_bit(WDM_READ, &desc->flags);
rv = service_outstanding_interrupt(desc);
spin_unlock_irq(&desc->iuspin);
if (rv < 0)
goto err;
Expand All @@ -584,8 +601,10 @@ static ssize_t wdm_read

desc->length -= cntr;
/* in case we had outstanding data */
if (!desc->length)
clear_wdm_read_flag(desc);
if (!desc->length) {
clear_bit(WDM_READ, &desc->flags);
service_outstanding_interrupt(desc);
}
spin_unlock_irq(&desc->iuspin);
rv = cntr;

Expand Down

0 comments on commit c1da59d

Please sign in to comment.