lkml.org 
[lkml]   [2020]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.9 248/391] usb: cdc-acm: fix cooldown mechanism
    Date
    From: Jerome Brunet <jbrunet@baylibre.com>

    commit 38203b8385bf6283537162bde7d499f830964711 upstream.

    Commit a4e7279cd1d1 ("cdc-acm: introduce a cool down") is causing
    regression if there is some USB error, such as -EPROTO.

    This has been reported on some samples of the Odroid-N2 using the Combee II
    Zibgee USB dongle.

    > struct acm *acm = container_of(work, struct acm, work)

    is incorrect in case of a delayed work and causes warnings, usually from
    the workqueue:

    > WARNING: CPU: 0 PID: 0 at kernel/workqueue.c:1474 __queue_work+0x480/0x528.

    When this happens, USB eventually stops working completely after a while.
    Also the ACM_ERROR_DELAY bit is never set, so the cooldown mechanism
    previously introduced cannot be triggered and acm_submit_read_urb() is
    never called.

    This changes makes the cdc-acm driver use a single delayed work, fixing the
    pointer arithmetic in acm_softint() and set the ACM_ERROR_DELAY when the
    cooldown mechanism appear to be needed.

    Fixes: a4e7279cd1d1 ("cdc-acm: introduce a cool down")
    Cc: Oliver Neukum <oneukum@suse.com>
    Reported-by: Pascal Vizeli <pascal.vizeli@nabucasa.com>
    Acked-by: Oliver Neukum <oneukum@suse.com>
    Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
    Link: https://lore.kernel.org/r/20201019170702.150534-1-jbrunet@baylibre.com
    Cc: stable <stable@vger.kernel.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/usb/class/cdc-acm.c | 12 +++++-------
    drivers/usb/class/cdc-acm.h | 3 +--
    2 files changed, 6 insertions(+), 9 deletions(-)

    --- a/drivers/usb/class/cdc-acm.c
    +++ b/drivers/usb/class/cdc-acm.c
    @@ -507,6 +507,7 @@ static void acm_read_bulk_callback(struc
    "%s - cooling babbling device\n", __func__);
    usb_mark_last_busy(acm->dev);
    set_bit(rb->index, &acm->urbs_in_error_delay);
    + set_bit(ACM_ERROR_DELAY, &acm->flags);
    cooldown = true;
    break;
    default:
    @@ -532,7 +533,7 @@ static void acm_read_bulk_callback(struc

    if (stopped || stalled || cooldown) {
    if (stalled)
    - schedule_work(&acm->work);
    + schedule_delayed_work(&acm->dwork, 0);
    else if (cooldown)
    schedule_delayed_work(&acm->dwork, HZ / 2);
    return;
    @@ -562,13 +563,13 @@ static void acm_write_bulk(struct urb *u
    acm_write_done(acm, wb);
    spin_unlock_irqrestore(&acm->write_lock, flags);
    set_bit(EVENT_TTY_WAKEUP, &acm->flags);
    - schedule_work(&acm->work);
    + schedule_delayed_work(&acm->dwork, 0);
    }

    static void acm_softint(struct work_struct *work)
    {
    int i;
    - struct acm *acm = container_of(work, struct acm, work);
    + struct acm *acm = container_of(work, struct acm, dwork.work);

    if (test_bit(EVENT_RX_STALL, &acm->flags)) {
    smp_mb(); /* against acm_suspend() */
    @@ -584,7 +585,7 @@ static void acm_softint(struct work_stru
    if (test_and_clear_bit(ACM_ERROR_DELAY, &acm->flags)) {
    for (i = 0; i < acm->rx_buflimit; i++)
    if (test_and_clear_bit(i, &acm->urbs_in_error_delay))
    - acm_submit_read_urb(acm, i, GFP_NOIO);
    + acm_submit_read_urb(acm, i, GFP_KERNEL);
    }

    if (test_and_clear_bit(EVENT_TTY_WAKEUP, &acm->flags))
    @@ -1364,7 +1365,6 @@ made_compressed_probe:
    acm->ctrlsize = ctrlsize;
    acm->readsize = readsize;
    acm->rx_buflimit = num_rx_buf;
    - INIT_WORK(&acm->work, acm_softint);
    INIT_DELAYED_WORK(&acm->dwork, acm_softint);
    init_waitqueue_head(&acm->wioctl);
    spin_lock_init(&acm->write_lock);
    @@ -1574,7 +1574,6 @@ static void acm_disconnect(struct usb_in
    }

    acm_kill_urbs(acm);
    - cancel_work_sync(&acm->work);
    cancel_delayed_work_sync(&acm->dwork);

    tty_unregister_device(acm_tty_driver, acm->minor);
    @@ -1617,7 +1616,6 @@ static int acm_suspend(struct usb_interf
    return 0;

    acm_kill_urbs(acm);
    - cancel_work_sync(&acm->work);
    cancel_delayed_work_sync(&acm->dwork);
    acm->urbs_in_error_delay = 0;

    --- a/drivers/usb/class/cdc-acm.h
    +++ b/drivers/usb/class/cdc-acm.h
    @@ -112,8 +112,7 @@ struct acm {
    # define ACM_ERROR_DELAY 3
    unsigned long urbs_in_error_delay; /* these need to be restarted after a delay */
    struct usb_cdc_line_coding line; /* bits, stop, parity */
    - struct work_struct work; /* work queue entry for various purposes*/
    - struct delayed_work dwork; /* for cool downs needed in error recovery */
    + struct delayed_work dwork; /* work queue entry for various purposes */
    unsigned int ctrlin; /* input control lines (DCD, DSR, RI, break, overruns) */
    unsigned int ctrlout; /* output control lines (DTR, RTS) */
    struct async_icount iocount; /* counters for control line changes */

    \
     
     \ /
      Last update: 2020-11-03 21:48    [W:4.367 / U:0.420 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site