[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bckroyio6l2nt54refuord4pm6mqylt3adx6z2bg6iczxkbnyk@bb5447rqahj5>
Date: Tue, 20 Feb 2024 09:25:40 -0800
From: Davidlohr Bueso <dave@...olabs.net>
To: Tejun Heo <tj@...nel.org>
Cc: torvalds@...ux-foundation.org, mpatocka@...hat.com,
linux-kernel@...r.kernel.org, dm-devel@...ts.linux.dev, msnitzer@...hat.com,
ignat@...udflare.com, damien.lemoal@....com, bob.liu@...cle.com, houtao1@...wei.com,
peterz@...radead.org, mingo@...nel.org, netdev@...r.kernel.org, allen.lkml@...il.com,
kernel-team@...a.com, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Alan Stern <stern@...land.harvard.edu>, linux-usb@...r.kernel.org, mchehab@...nel.org
Subject: Re: [PATCH 5/8] usb: core: hcd: Convert from tasklet to BH workqueue
On Mon, 29 Jan 2024, Tejun Heo wrote:
>The only generic interface to execute asynchronously in the BH context is
>tasklet; however, it's marked deprecated and has some design flaws. To
>replace tasklets, BH workqueue support was recently added. A BH workqueue
>behaves similarly to regular workqueues except that the queued work items
>are executed in the BH context.
>
>This patch converts usb hcd from tasklet to BH workqueue.
In the past this tasklet removal was held up by Mauro's device not properly
streaming - hopefully this no longer the case. Cc'ing.
https://lore.kernel.org/all/20180216170450.yl5owfphuvltstnt@breakpoint.cc/
>
>Signed-off-by: Tejun Heo <tj@...nel.org>
>Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>Cc: Alan Stern <stern@...land.harvard.edu>
>Cc: linux-usb@...r.kernel.org
Acked-by: Davidlohr Bueso <dave@...olabs.net>
>---
> drivers/usb/core/hcd.c | 23 ++++++++++++-----------
> include/linux/usb/hcd.h | 2 +-
> 2 files changed, 13 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>index 12b6dfeaf658..edf74458474a 100644
>--- a/drivers/usb/core/hcd.c
>+++ b/drivers/usb/core/hcd.c
>@@ -1664,9 +1664,10 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
> usb_put_urb(urb);
> }
>
>-static void usb_giveback_urb_bh(struct tasklet_struct *t)
>+static void usb_giveback_urb_bh(struct work_struct *work)
> {
>- struct giveback_urb_bh *bh = from_tasklet(bh, t, bh);
>+ struct giveback_urb_bh *bh =
>+ container_of(work, struct giveback_urb_bh, bh);
> struct list_head local_list;
>
> spin_lock_irq(&bh->lock);
>@@ -1691,9 +1692,9 @@ static void usb_giveback_urb_bh(struct tasklet_struct *t)
> spin_lock_irq(&bh->lock);
> if (!list_empty(&bh->head)) {
> if (bh->high_prio)
>- tasklet_hi_schedule(&bh->bh);
>+ queue_work(system_bh_highpri_wq, &bh->bh);
> else
>- tasklet_schedule(&bh->bh);
>+ queue_work(system_bh_wq, &bh->bh);
> }
> bh->running = false;
> spin_unlock_irq(&bh->lock);
>@@ -1706,7 +1707,7 @@ static void usb_giveback_urb_bh(struct tasklet_struct *t)
> * @status: completion status code for the URB.
> *
> * Context: atomic. The completion callback is invoked in caller's context.
>- * For HCDs with HCD_BH flag set, the completion callback is invoked in tasklet
>+ * For HCDs with HCD_BH flag set, the completion callback is invoked in BH
> * context (except for URBs submitted to the root hub which always complete in
> * caller's context).
> *
>@@ -1725,7 +1726,7 @@ void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb, int status)
> struct giveback_urb_bh *bh;
> bool running;
>
>- /* pass status to tasklet via unlinked */
>+ /* pass status to BH via unlinked */
> if (likely(!urb->unlinked))
> urb->unlinked = status;
>
>@@ -1747,9 +1748,9 @@ void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb, int status)
> if (running)
> ;
> else if (bh->high_prio)
>- tasklet_hi_schedule(&bh->bh);
>+ queue_work(system_bh_highpri_wq, &bh->bh);
> else
>- tasklet_schedule(&bh->bh);
>+ queue_work(system_bh_wq, &bh->bh);
> }
> EXPORT_SYMBOL_GPL(usb_hcd_giveback_urb);
>
>@@ -2540,7 +2541,7 @@ static void init_giveback_urb_bh(struct giveback_urb_bh *bh)
>
> spin_lock_init(&bh->lock);
> INIT_LIST_HEAD(&bh->head);
>- tasklet_setup(&bh->bh, usb_giveback_urb_bh);
>+ INIT_WORK(&bh->bh, usb_giveback_urb_bh);
> }
>
> struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver,
>@@ -2926,7 +2927,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
> && device_can_wakeup(&hcd->self.root_hub->dev))
> dev_dbg(hcd->self.controller, "supports USB remote wakeup\n");
>
>- /* initialize tasklets */
>+ /* initialize BHs */
> init_giveback_urb_bh(&hcd->high_prio_bh);
> hcd->high_prio_bh.high_prio = true;
> init_giveback_urb_bh(&hcd->low_prio_bh);
>@@ -3036,7 +3037,7 @@ void usb_remove_hcd(struct usb_hcd *hcd)
> mutex_unlock(&usb_bus_idr_lock);
>
> /*
>- * tasklet_kill() isn't needed here because:
>+ * flush_work() isn't needed here because:
> * - driver's disconnect() called from usb_disconnect() should
> * make sure its URBs are completed during the disconnect()
> * callback
>diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
>index 00724b4f6e12..f698aac71de3 100644
>--- a/include/linux/usb/hcd.h
>+++ b/include/linux/usb/hcd.h
>@@ -55,7 +55,7 @@ struct giveback_urb_bh {
> bool high_prio;
> spinlock_t lock;
> struct list_head head;
>- struct tasklet_struct bh;
>+ struct work_struct bh;
> struct usb_host_endpoint *completing_ep;
> };
>
>--
>2.43.0
>
Powered by blists - more mailing lists