* [PATCH 5/8] usb: core: hcd: Convert from tasklet to BH workqueue [not found] <20240130091300.2968534-1-tj@kernel.org> @ 2024-01-30 9:11 ` Tejun Heo 2024-01-30 16:38 ` Greg Kroah-Hartman 2024-02-20 17:25 ` Davidlohr Bueso 0 siblings, 2 replies; 6+ messages in thread From: Tejun Heo @ 2024-01-30 9:11 UTC (permalink / raw) To: torvalds, mpatocka Cc: linux-kernel, dm-devel, msnitzer, ignat, damien.lemoal, bob.liu, houtao1, peterz, mingo, netdev, allen.lkml, kernel-team, Tejun Heo, Greg Kroah-Hartman, Alan Stern, linux-usb 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. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Alan Stern <stern@rowland.harvard.edu> Cc: linux-usb@vger.kernel.org --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 5/8] usb: core: hcd: Convert from tasklet to BH workqueue 2024-01-30 9:11 ` [PATCH 5/8] usb: core: hcd: Convert from tasklet to BH workqueue Tejun Heo @ 2024-01-30 16:38 ` Greg Kroah-Hartman 2024-02-20 17:25 ` Davidlohr Bueso 1 sibling, 0 replies; 6+ messages in thread From: Greg Kroah-Hartman @ 2024-01-30 16:38 UTC (permalink / raw) To: Tejun Heo Cc: torvalds, mpatocka, linux-kernel, dm-devel, msnitzer, ignat, damien.lemoal, bob.liu, houtao1, peterz, mingo, netdev, allen.lkml, kernel-team, Alan Stern, linux-usb On Mon, Jan 29, 2024 at 11:11:52PM -1000, 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. > > Signed-off-by: Tejun Heo <tj@kernel.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Alan Stern <stern@rowland.harvard.edu> > Cc: linux-usb@vger.kernel.org > --- > drivers/usb/core/hcd.c | 23 ++++++++++++----------- > include/linux/usb/hcd.h | 2 +- > 2 files changed, 13 insertions(+), 12 deletions(-) Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/8] usb: core: hcd: Convert from tasklet to BH workqueue 2024-01-30 9:11 ` [PATCH 5/8] usb: core: hcd: Convert from tasklet to BH workqueue Tejun Heo 2024-01-30 16:38 ` Greg Kroah-Hartman @ 2024-02-20 17:25 ` Davidlohr Bueso 2024-02-20 17:55 ` Linus Torvalds 1 sibling, 1 reply; 6+ messages in thread From: Davidlohr Bueso @ 2024-02-20 17:25 UTC (permalink / raw) To: Tejun Heo Cc: torvalds, mpatocka, linux-kernel, dm-devel, msnitzer, ignat, damien.lemoal, bob.liu, houtao1, peterz, mingo, netdev, allen.lkml, kernel-team, Greg Kroah-Hartman, Alan Stern, linux-usb, mchehab 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@kernel.org> >Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >Cc: Alan Stern <stern@rowland.harvard.edu> >Cc: linux-usb@vger.kernel.org Acked-by: Davidlohr Bueso <dave@stgolabs.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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/8] usb: core: hcd: Convert from tasklet to BH workqueue 2024-02-20 17:25 ` Davidlohr Bueso @ 2024-02-20 17:55 ` Linus Torvalds 2024-02-20 18:19 ` Tejun Heo 2024-02-20 19:36 ` Davidlohr Bueso 0 siblings, 2 replies; 6+ messages in thread From: Linus Torvalds @ 2024-02-20 17:55 UTC (permalink / raw) To: Tejun Heo, torvalds, mpatocka, linux-kernel, dm-devel, msnitzer, ignat, damien.lemoal, bob.liu, houtao1, peterz, mingo, netdev, allen.lkml, kernel-team, Greg Kroah-Hartman, Alan Stern, linux-usb, mchehab On Tue, 20 Feb 2024 at 09:25, Davidlohr Bueso <dave@stgolabs.net> wrote: > > 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/ Oh, lovely - an actual use-case where the old tasklet code has known requirements. Mauro - the BH workqueue should provide the same kind of latency as the tasklets, and it would be good to validate early that yes, this workqueue conversion works well in practice. Since you have an actual real-life test-case, could you give it a try? You can find the work in git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git refs/heads/for-6.9-bh-conversions although it's possible that Tejun has a newer version in some other branch. Tejun - maybe point Mauro at something he can try out if you have updated the conversion since? Linus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/8] usb: core: hcd: Convert from tasklet to BH workqueue 2024-02-20 17:55 ` Linus Torvalds @ 2024-02-20 18:19 ` Tejun Heo 2024-02-20 19:36 ` Davidlohr Bueso 1 sibling, 0 replies; 6+ messages in thread From: Tejun Heo @ 2024-02-20 18:19 UTC (permalink / raw) To: Linus Torvalds Cc: mpatocka, linux-kernel, dm-devel, msnitzer, ignat, damien.lemoal, bob.liu, houtao1, peterz, mingo, netdev, allen.lkml, kernel-team, Greg Kroah-Hartman, Alan Stern, linux-usb, mchehab Hello, On Tue, Feb 20, 2024 at 09:55:30AM -0800, Linus Torvalds wrote: > git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git > refs/heads/for-6.9-bh-conversions > > although it's possible that Tejun has a newer version in some other > branch. Tejun - maybe point Mauro at something he can try out if you > have updated the conversion since? Just pushed out the following branch for testing. git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.9-bh-conversions-test It's the same branch but combined with the current linus#master to avoid the rc1 wonkiness. Thanks. -- tejun ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/8] usb: core: hcd: Convert from tasklet to BH workqueue 2024-02-20 17:55 ` Linus Torvalds 2024-02-20 18:19 ` Tejun Heo @ 2024-02-20 19:36 ` Davidlohr Bueso 1 sibling, 0 replies; 6+ messages in thread From: Davidlohr Bueso @ 2024-02-20 19:36 UTC (permalink / raw) To: Linus Torvalds Cc: Tejun Heo, mpatocka, linux-kernel, dm-devel, msnitzer, ignat, damien.lemoal, bob.liu, houtao1, peterz, mingo, netdev, allen.lkml, kernel-team, Greg Kroah-Hartman, Alan Stern, linux-usb, mchehab On Tue, 20 Feb 2024, Linus Torvalds wrote: >Mauro - the BH workqueue should provide the same kind of latency as >the tasklets, and it would be good to validate early that yes, this >workqueue conversion works well in practice. Since you have an actual >real-life test-case, could you give it a try? In general I think it's worth pointing out that future conversions should still aim for an equivalent in task context, and now with disable/enable_work a lot opens up for regular wq conversions. If users/maintainers shout about latency, then use BH wq. Thanks, Davidlohr ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-02-20 19:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240130091300.2968534-1-tj@kernel.org>
2024-01-30 9:11 ` [PATCH 5/8] usb: core: hcd: Convert from tasklet to BH workqueue Tejun Heo
2024-01-30 16:38 ` Greg Kroah-Hartman
2024-02-20 17:25 ` Davidlohr Bueso
2024-02-20 17:55 ` Linus Torvalds
2024-02-20 18:19 ` Tejun Heo
2024-02-20 19:36 ` Davidlohr Bueso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox