| From 26c6c2f8a907c9e3a2f24990552a4d77235791e6 Mon Sep 17 00:00:00 2001 |
| From: Weitao Wang <WeitaoWang-oc@zhaoxin.com> |
| Date: Tue, 26 Jul 2022 15:49:18 +0800 |
| Subject: USB: HCD: Fix URB giveback issue in tasklet function |
| |
| From: Weitao Wang <WeitaoWang-oc@zhaoxin.com> |
| |
| commit 26c6c2f8a907c9e3a2f24990552a4d77235791e6 upstream. |
| |
| 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") |
| Cc: stable <stable@kernel.org> |
| Reviewed-by: Alan Stern <stern@rowland.harvard.edu> |
| Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com> |
| Link: https://lore.kernel.org/r/20220726074918.5114-1-WeitaoWang-oc@zhaoxin.com |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/usb/core/hcd.c | 26 +++++++++++++++----------- |
| include/linux/usb/hcd.h | 1 + |
| 2 files changed, 16 insertions(+), 11 deletions(-) |
| |
| --- a/drivers/usb/core/hcd.c |
| +++ b/drivers/usb/core/hcd.c |
| @@ -1688,7 +1688,6 @@ static void usb_giveback_urb_bh(unsigned |
| |
| spin_lock_irq(&bh->lock); |
| bh->running = true; |
| - restart: |
| list_replace_init(&bh->head, &local_list); |
| spin_unlock_irq(&bh->lock); |
| |
| @@ -1702,10 +1701,17 @@ static void usb_giveback_urb_bh(unsigned |
| 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); |
| } |
| @@ -1730,7 +1736,7 @@ static void usb_giveback_urb_bh(unsigned |
| 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)) |
| @@ -1741,13 +1747,10 @@ void usb_hcd_giveback_urb(struct usb_hcd |
| 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); |
| @@ -1756,7 +1759,7 @@ void usb_hcd_giveback_urb(struct usb_hcd |
| |
| if (running) |
| ; |
| - else if (high_prio_bh) |
| + else if (bh->high_prio) |
| tasklet_hi_schedule(&bh->bh); |
| else |
| tasklet_schedule(&bh->bh); |
| @@ -2796,6 +2799,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, |
| --- 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; |