[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZYFaeR-83eYNWQIz@localhost.localdomain>
Date: Tue, 19 Dec 2023 09:55:21 +0100
From: Juri Lelli <juri.lelli@...hat.com>
To: Tejun Heo <tj@...nel.org>
Cc: Aaron Tomlin <atomlin@...mlin.com>, linux-kernel@...r.kernel.org,
jiangshanlai@...il.com, peterz@...radead.org
Subject: Re: [RFC PATCH 0/2] workqueue: Introduce PF_WQ_RESCUE_WORKER
Hello again,
On 15/12/23 07:50, Juri Lelli wrote:
> On 14/12/23 09:47, Tejun Heo wrote:
> > Hello,
> >
> > On Thu, Dec 14, 2023 at 12:25:25PM +0100, Juri Lelli wrote:
> > > > So, we have to use set_cpus_allowed_ptr() but we still don't want to change
> > > > the affinity of a rescuer which is already running a task for a pool.
> > >
> > > But then, even today, a rescuer might keep handling work on a cpu
> > > outside its wq cpumask if the associated wq cpumask change can proceed
> > > w/o waiting for it to finish the iteration?
> >
> > Yeah, that can happen and pool cpumasks naturally being subsets of the wq's
> > cpumask that they're serving, your original approach likely isn't broken
> > either.
> >
> > > BTW, apologies for all the questions, but I'd like to make sure I can
> > > get the implications hopefully right. :)
> >
> > I obviously haven't thought through it very well, so thanks for the
> > questions. So, yeah, I think we actually need to set the rescuer's cpumask
> > when wq's cpumask changes and doing it where you were suggesting should
> > probably work.
>
> OK. Going to send a proper patch asap.
I actually didn't do that yet as it turns out the proposed approach
doesn't cover !WQ_SYSFS unbounded wqs. Well, I thought those should be
covered as well, since we have (initiated by echo <mask> into
/sys/devices/virtual/workqueue/cpumask)
workqueue_apply_unbound_cpumask ->
apply_wqattrs_commit
but for some reason the mask change is not reflected into rescuers
affinity.
Trying to dig deeper I went ahead and extended the recent wq_dump.py
addition with the following
---
ls/workqueue/wq_dump.py | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/tools/workqueue/wq_dump.py b/tools/workqueue/wq_dump.py
index d0df5833f2c18..6da621989e210 100644
--- a/tools/workqueue/wq_dump.py
+++ b/tools/workqueue/wq_dump.py
@@ -175,3 +175,32 @@ for wq in list_for_each_entry('struct workqueue_struct', workqueues.address_of_(
if wq.flags & WQ_UNBOUND:
print(f' {wq.dfl_pwq.pool.id.value_():{max_pool_id_len}}', end='')
print('')
+
+print('')
+print('Workqueue -> rescuer')
+print('=====================')
+print(f'wq_unbound_cpumask={cpumask_str(wq_unbound_cpumask)}')
+print('')
+print('[ workqueue \ type unbound_cpumask rescuer pid cpumask]')
+
+for wq in list_for_each_entry('struct workqueue_struct', workqueues.address_of_(), 'list'):
+ print(f'{wq.name.string_().decode()[-24:]:24}', end='')
+ if wq.flags & WQ_UNBOUND:
+ if wq.flags & WQ_ORDERED:
+ print(' ordered ', end='')
+ else:
+ print(' unbound', end='')
+ if wq.unbound_attrs.affn_strict:
+ print(',S ', end='')
+ else:
+ print(' ', end='')
+ print(f' {cpumask_str(wq.unbound_attrs.cpumask):24}', end='')
+ else:
+ print(' percpu ', end='')
+ print(' ', end='')
+
+ if wq.flags & WQ_MEM_RECLAIM:
+ print(f' {wq.rescuer.task.comm.string_().decode()[-24:]:24}', end='')
+ print(f' {wq.rescuer.task.pid.value_():5}', end='')
+ print(f' {cpumask_str(wq.rescuer.task.cpus_ptr)}', end='')
+ print('')
---
which shows the following situation after an
# echo 00,00000003 > /sys/devices/virtual/workqueue/cpumask
on the system I'm testing with:
...
Workqueue -> rescuer
=====================
wq_unbound_cpumask=00000003
[ workqueue \ type unbound_cpumask rescuer pid cpumask]
events percpu
events_highpri percpu
events_long percpu
events_unbound unbound 0xffffffff 000000ff
events_freezable percpu
events_power_efficient percpu
events_freezable_power_ percpu
rcu_gp percpu kworker/R-rcu_g 4 0xffffffff 000000ff
rcu_par_gp percpu kworker/R-rcu_p 5 0xffffffff 000000ff
slub_flushwq percpu kworker/R-slub_ 6 0xffffffff 000000ff
netns ordered 0xffffffff 000000ff kworker/R-netns 7 0xffffffff 000000ff
mm_percpu_wq percpu kworker/R-mm_pe 13 0xffffffff 000000ff
cpuset_migrate_mm ordered 0xffffffff 000000ff
inet_frag_wq percpu kworker/R-inet_ 300 0xffffffff 000000ff
pm percpu
cgroup_destroy percpu
cgroup_pidlist_destroy percpu
writeback unbound 0xffffffff 000000ff kworker/R-write 308 0xffffffff 000000ff
cgwb_release percpu
cryptd percpu kworker/R-crypt 314 0xffffffff 000000ff
kintegrityd percpu kworker/R-kinte 315 0xffffffff 000000ff
kblockd percpu kworker/R-kbloc 316 0xffffffff 000000ff
kacpid percpu
kacpi_notify percpu
kacpi_hotplug ordered 0xffffffff 000000ff
kec ordered 0xffffffff 000000ff
kec_query percpu
tpm_dev_wq percpu kworker/R-tpm_d 352 0xffffffff 000000ff
usb_hub_wq percpu
md percpu kworker/R-md 353 0xffffffff 000000ff
md_misc percpu
md_bitmap unbound 0xffffffff 000000ff kworker/R-md_bi 354 0xffffffff 000000ff
edac-poller ordered 0xffffffff 000000ff kworker/R-edac- 355 0xffffffff 000000ff
...
I guess I expected wq_unbound_cpumask and unbound_cpumask for each
unbound wq to be kept in sync, so I'm evidently missing details. :)
Can you please help me here understanding what am I missing?
Thanks!
Juri
Powered by blists - more mailing lists