On Tue, Jul 26, 2022 at 03:49:18PM +0800, Weitao Wang wrote:
> Usb core introduce the mechanism of giveback of URB in tasklet context to
> reduce hardware interrupt handling time. On some test situation(such as
> FIO with 4KB block size), when tasklet callback function called to
> giveback URB, interrupt handler add URB node to the bh->head list also.
> If check bh->head list again after finish all URB giveback of local_list,
> then it may introduce a "dynamic balance" between giveback URB and add URB
> to bh->head list. This tasklet callback function may not exit for a long
> time, which will cause other tasklet function calls to be delayed. Some
> real-time applications(such as KB and Mouse) will see noticeable lag.
>
> In order to prevent the tasklet function from occupying the cpu for a long
> time at a time, new URBS will not be added to the local_list even though
> the bh->head list is not empty. But also need to ensure the left URB
> giveback to be processed in time, so add a member high_prio for structure
> giveback_urb_bh to prioritize tasklet and schelule this tasklet again if
> bh->head list is not empty.
>
> At the same time, we are able to prioritize tasklet through structure
> member high_prio. So, replace the local high_prio_bh variable with this
> structure member in usb_hcd_giveback_urb.
>
> Fixes: 94dfd7edfd5c ("USB: HCD: support giveback of URB in tasklet context")
> Signed-off-by: Weitao Wang <WeitaoWang-oc@xxxxxxxxxxx>
> ---
Reviewed-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> v2->v3
> - Add more detail info about how to patch this issue.
> - Change initial value of boolean variable high_prio from 1 to true.
>
> drivers/usb/core/hcd.c | 26 +++++++++++++++-----------
> include/linux/usb/hcd.h | 1 +
> 2 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 06eea8848ccc..11c8ea0cccc8 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1691,7 +1691,6 @@ static void usb_giveback_urb_bh(struct tasklet_struct *t)
>
> spin_lock_irq(&bh->lock);
> bh->running = true;
> - restart:
> list_replace_init(&bh->head, &local_list);
> spin_unlock_irq(&bh->lock);
>
> @@ -1705,10 +1704,17 @@ static void usb_giveback_urb_bh(struct tasklet_struct *t)
> bh->completing_ep = NULL;
> }
>
> - /* check if there are new URBs to giveback */
> + /*
> + * giveback new URBs next time to prevent this function
> + * from not exiting for a long time.
> + */
> spin_lock_irq(&bh->lock);
> - if (!list_empty(&bh->head))
> - goto restart;
> + if (!list_empty(&bh->head)) {
> + if (bh->high_prio)
> + tasklet_hi_schedule(&bh->bh);
> + else
> + tasklet_schedule(&bh->bh);
> + }
> bh->running = false;
> spin_unlock_irq(&bh->lock);
> }
> @@ -1737,7 +1743,7 @@ static void usb_giveback_urb_bh(struct tasklet_struct *t)
> void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb, int status)
> {
> struct giveback_urb_bh *bh;
> - bool running, high_prio_bh;
> + bool running;
>
> /* pass status to tasklet via unlinked */
> if (likely(!urb->unlinked))
> @@ -1748,13 +1754,10 @@ void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb, int status)
> return;
> }
>
> - if (usb_pipeisoc(urb->pipe) || usb_pipeint(urb->pipe)) {
> + if (usb_pipeisoc(urb->pipe) || usb_pipeint(urb->pipe))
> bh = &hcd->high_prio_bh;
> - high_prio_bh = true;
> - } else {
> + else
> bh = &hcd->low_prio_bh;
> - high_prio_bh = false;
> - }
>
> spin_lock(&bh->lock);
> list_add_tail(&urb->urb_list, &bh->head);
> @@ -1763,7 +1766,7 @@ void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb, int status)
>
> if (running)
> ;
> - else if (high_prio_bh)
> + else if (bh->high_prio)
> tasklet_hi_schedule(&bh->bh);
> else
> tasklet_schedule(&bh->bh);
> @@ -2959,6 +2962,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>
> /* initialize tasklets */
> init_giveback_urb_bh(&hcd->high_prio_bh);
> + hcd->high_prio_bh.high_prio = true;
> init_giveback_urb_bh(&hcd->low_prio_bh);
>
> /* enable irqs just before we start the controller,
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index 2c1fc9212cf2..98d1921f02b1 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -66,6 +66,7 @@
>
> struct giveback_urb_bh {
> bool running;
> + bool high_prio;
> spinlock_t lock;
> struct list_head head;
> struct tasklet_struct bh;
> --
> 2.32.0
