From: Tejun Heo <tj@kernel.org>
To: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: torvalds@linux-foundation.org, mpatocka@redhat.com,
linux-kernel@vger.kernel.org, dm-devel@lists.linux.dev,
msnitzer@redhat.com, ignat@cloudflare.com, damien.lemoal@wdc.com,
bob.liu@oracle.com, houtao1@huawei.com, peterz@infradead.org,
mingo@kernel.org, netdev@vger.kernel.org, allen.lkml@gmail.com,
kernel-team@meta.com
Subject: Re: [PATCH 3/8] workqueue: Implement BH workqueues to eventually replace tasklets
Date: Thu, 1 Feb 2024 11:47:40 -1000 [thread overview]
Message-ID: <ZbwRfHCfss2THOeX@slm.duckdns.org> (raw)
In-Reply-To: <CAJhGHyCk_cjD-Ex_OAd=DrBkTEe0Xvs81=On65Sp7sS8zNBfGQ@mail.gmail.com>
Hello, Lai.
On Thu, Feb 01, 2024 at 07:02:27PM +0800, Lai Jiangshan wrote:
> On Tue, Jan 30, 2024 at 5:16 PM Tejun Heo <tj@kernel.org> wrote:
> > @@ -1184,6 +1211,14 @@ static bool kick_pool(struct worker_pool *pool)
> > if (!need_more_worker(pool) || !worker)
> > return false;
> >
> > + if (pool->flags & POOL_BH) {
> > + if (pool->attrs->nice == HIGHPRI_NICE_LEVEL)
> > + tasklet_hi_schedule(&worker->bh_tasklet);
> > + else
> > + tasklet_schedule(&worker->bh_tasklet);
> > + return true;
> > + }
>
> I think it is more straightforward to call bh_worker_taskletfn[_hi]()
> in tasklet_action() and tasklet_hi_action() rather than add a
> worker->bh_tasklet.
>
> raise_softirq_irqoff() can be used here (kick_pool()) instead.
>
> As the changelog said, once the conversion is complete, tasklet can be
> removed and BH workqueues can directly take over the tasklet softirqs,
> in which case, then, bh_worker_taskletfn[_hi]() can directly take over
> the tasklet_action() and tasklet_hi_action().
Hmmm.... maybe. Yeah, that'd also make it a tiny bit cheaper for hot paths.
Lemme see how that looks.
> I think wq->max_active can be forced to be UINT_MAX or ULONG_MAX
> in the max_active management code to avoid a branch here.
Good point. Will update.
> worker_attach_to_pool() and worker_detach_from_pool also access to
> worker->task with kthread_set_per_cpu() and luckily to_kthread()
> checks the NULL pointer for it.
>
> IMO, it is better to avoid calling kthread_set_per_cpu() for bh workers.
Note that BH worker pools are always DISASSOCIATED, so
worker_attach_to_pool() shouldn't call kthread_set_per_cpu(). Also, BH
workers and worker pools are never destroyed, so worker_detach_from_pool()
shouldn't be called either. I'll add WARN_ONs to clarify.
> > @@ -5605,7 +5731,12 @@ static void pr_cont_pool_info(struct worker_pool *pool)
> > pr_cont(" cpus=%*pbl", nr_cpumask_bits, pool->attrs->cpumask);
> > if (pool->node != NUMA_NO_NODE)
> > pr_cont(" node=%d", pool->node);
> > - pr_cont(" flags=0x%x nice=%d", pool->flags, pool->attrs->nice);
> > + pr_cont(" flags=0x%x", pool->flags);
> > + if (pool->flags & POOL_BH)
> > + pr_cont(" bh%s",
> > + pool->attrs->nice == HIGHPRI_NICE_LEVEL ? "-hi" : "");
> > + else
> > + pr_cont(" nice=%d", pool->attrs->nice);
>
> There are also some "worker->task" in show_pwq(), show_one_worker_pool(),
> and show_cpu_pool_hog() needing taking care of.
Ah, right, I'll hunt them down.
Thanks.
--
tejun
next prev parent reply other threads:[~2024-02-01 21:47 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-30 9:11 [PATCHSET wq/for-6.9] workqueue: Implement BH workqueue and convert several tasklet users Tejun Heo
2024-01-30 9:11 ` [PATCH 1/8] workqueue: Update lock debugging code Tejun Heo
2024-01-30 9:11 ` [PATCH 2/8] workqueue: Factor out init_cpu_worker_pool() Tejun Heo
2024-01-30 9:11 ` [PATCH 3/8] workqueue: Implement BH workqueues to eventually replace tasklets Tejun Heo
2024-01-30 17:25 ` Linus Torvalds
2024-02-01 11:02 ` Lai Jiangshan
2024-02-01 21:47 ` Tejun Heo [this message]
2024-02-02 1:15 ` [PATCH v2 " Tejun Heo
2024-02-04 2:20 ` Lai Jiangshan
2024-02-04 21:29 ` [PATCH v3 " Tejun Heo
2024-02-05 4:48 ` Hillf Danton
2024-02-05 17:47 ` Tejun Heo
2024-02-26 2:00 ` Boqun Feng
2024-02-26 18:47 ` Tejun Heo
2024-02-27 1:38 ` [PATCH for-6.9] workqueue: Drain BH work items on hot-unplugged CPUs Tejun Heo
2024-02-29 20:37 ` Tejun Heo
2024-02-29 21:07 ` Boqun Feng
2024-01-30 9:11 ` [PATCH 4/8] backtracetest: Convert from tasklet to BH workqueue Tejun Heo
2024-01-30 9:11 ` [PATCH 5/8] usb: core: hcd: " 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
2024-01-30 9:11 ` [PATCH 6/8] net: tcp: tsq: " Tejun Heo
2024-02-16 5:31 ` Tejun Heo
2024-02-16 8:23 ` Eric Dumazet
2024-02-16 15:52 ` David Wei
2024-02-16 16:24 ` Tejun Heo
2024-01-30 9:11 ` [PATCH 7/8] dm-crypt: " Tejun Heo
2024-01-30 10:46 ` Sebastian Andrzej Siewior
2024-01-30 15:53 ` Tejun Heo
2024-01-31 21:23 ` Mikulas Patocka
2024-01-30 9:11 ` [PATCH 8/8] dm-verity: " Tejun Heo
2024-01-31 21:19 ` Mikulas Patocka
2024-01-31 21:32 ` Tejun Heo
2024-01-31 22:02 ` Mikulas Patocka
2024-01-31 23:19 ` Linus Torvalds
2024-02-01 0:04 ` Tejun Heo
2024-02-01 0:19 ` Mike Snitzer
2024-02-20 19:44 ` Mike Snitzer
2024-02-20 20:05 ` Tejun Heo
2024-02-22 21:24 ` Mike Snitzer
2024-02-23 17:22 ` Tejun Heo
2024-02-01 0:07 ` Mike Snitzer
2024-01-30 9:22 ` [PATCHSET wq/for-6.9] workqueue: Implement BH workqueue and convert several tasklet users Tejun Heo
2024-01-30 10:20 ` Sebastian Andrzej Siewior
2024-01-30 15:50 ` Tejun Heo
2024-01-30 23:37 ` Allen
2024-02-04 21:33 ` Tejun Heo
2024-02-05 20:50 ` Allen
2024-02-05 21:12 ` Tejun Heo
2024-02-05 21:31 ` Allen
2024-02-07 19:02 ` Allen
2024-02-08 16:56 ` Tejun Heo
2024-02-08 19:07 ` Allen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZbwRfHCfss2THOeX@slm.duckdns.org \
--to=tj@kernel.org \
--cc=allen.lkml@gmail.com \
--cc=bob.liu@oracle.com \
--cc=damien.lemoal@wdc.com \
--cc=dm-devel@lists.linux.dev \
--cc=houtao1@huawei.com \
--cc=ignat@cloudflare.com \
--cc=jiangshanlai@gmail.com \
--cc=kernel-team@meta.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=mpatocka@redhat.com \
--cc=msnitzer@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).