All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Lai Jiangshan <jiangshanlai@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Valentin Schneider <valentin.schneider@arm.com>,
	Qian Cai <cai@redhat.com>,
	Vincent Donnefort <vincent.donnefort@arm.com>,
	Dexuan Cui <decui@microsoft.com>,
	Lai Jiangshan <laijs@linux.alibaba.com>,
	Tejun Heo <tj@kernel.org>
Subject: Re: [PATCH -tip V3 3/8] workqueue: introduce wq_online_cpumask
Date: Tue, 5 Jan 2021 16:23:44 +0800	[thread overview]
Message-ID: <CAJhGHyCwyuzikMZAxub=rxn9oe-N2P5C8CEOmyigd9d55SV5YA@mail.gmail.com> (raw)
In-Reply-To: <CAJhGHyB_MUHG8GGANcN9sQbjY7M5m8WPHQgXp-PmkGK481M5Tg@mail.gmail.com>

On Tue, Jan 5, 2021 at 10:41 AM Lai Jiangshan <jiangshanlai@gmail.com> wrote:
>
> On Mon, Jan 4, 2021 at 9:56 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Sat, Dec 26, 2020 at 10:51:11AM +0800, Lai Jiangshan wrote:
> > > From: Lai Jiangshan <laijs@linux.alibaba.com>
> > >
> > > wq_online_cpumask is the cached result of cpu_online_mask with the
> > > going-down cpu cleared.
> >
> > You can't use cpu_active_mask ?
>
>
> When a cpu is going out:
> (cpu_active_mask is not protected by workqueue mutexs.)
>
> create_worker() for unbound pool  |  cpu offlining
> check cpu_active_mask             |
check wq_online_cpumask
>                                   |  remove bit from cpu_active_mask
>                                   |  no cpu in pool->attrs->cpumask is active
> set pool->attrs->cpumask to worker|
> and hit the warning
                                    |  remove bit from wq_online_cpumask

Even with the help of wq_online_cpumask, the patchset can't silence
the warning in __set_cpus_allowed_ptr() in this case.  It is indeed
hard to suppress the warning for unbound pools.  Maybe we need something
like this (outmost callback of CPUHP_AP_WORKQUEUE_UNBOUND_ONLINE,
so that workqueue can do preparation when offlining before AP_ACTIVE):

diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 0042ef362511..ac2103deb20b 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -20,6 +20,9 @@
  *               |                               ^
  *               v                               |
  *              AP_ACTIVE                      AP_ACTIVE
+ *               |                               ^
+ *               v                               |
+ *              ONLINE                         ONLINE
  */

 enum cpuhp_state {
@@ -194,6 +197,7 @@ enum cpuhp_state {
        CPUHP_AP_X86_HPET_ONLINE,
        CPUHP_AP_X86_KVM_CLK_ONLINE,
        CPUHP_AP_ACTIVE,
+       CPUHP_AP_WORKQUEUE_UNBOUND_ONLINE,
        CPUHP_ONLINE,
 };


The other way is to modify __set_cpus_allowed_ptr() to suppress the
warning for kworkers and believe/let the workqueue handle cpumask correctly.

And the third way is to use get_online_cpus() in worker_attach_to_pool()
which might delay work items to be processed during cpuhotplug and might
be dangerous when someone call flush_work() in cpuhotplug callbacks.

Any thoughts?

Thanks,
Lai

>
>
> And when a cpu is onlining, there may be some workers which were just created
> after the workqueue hotplug callback is finished but before cpu_active_mask
> was updated. workqueue has not call back after cpu_active_mask updated and
> these workers' cpumask is not updated.
>
> For percpu workers, these problems can be handled with the help of
> POOL_DISASSOCIATED which is protected by workqueue mutexs and the
> help of sched/core.c which doesn't warn when per-cpu-kthread.
>
> For unbound workers, the way to handle it without using wq_online_cpumask
> is much more complex when a cpu is going out.

  parent reply	other threads:[~2021-01-05  8:24 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-26  2:51 [PATCH -tip V3 0/8] workqueue: break affinity initiatively Lai Jiangshan
2020-12-26  2:51 ` [PATCH -tip V3 1/8] workqueue: use cpu_possible_mask instead of cpu_active_mask to break affinity Lai Jiangshan
2020-12-26  2:51 ` [PATCH -tip V3 2/8] workqueue: Manually break affinity on pool detachment Lai Jiangshan
2020-12-26  2:51 ` [PATCH -tip V3 3/8] workqueue: introduce wq_online_cpumask Lai Jiangshan
2021-01-04 13:56   ` Peter Zijlstra
2021-01-05  2:41     ` Lai Jiangshan
2021-01-05  2:53       ` Lai Jiangshan
2021-01-05  8:23       ` Lai Jiangshan [this message]
2021-01-05 13:17         ` Peter Zijlstra
2021-01-05 14:37           ` Lai Jiangshan
2021-01-05 14:40             ` Lai Jiangshan
2021-01-05 16:24         ` Peter Zijlstra
2020-12-26  2:51 ` [PATCH -tip V3 4/8] workqueue: use wq_online_cpumask in restore_unbound_workers_cpumask() Lai Jiangshan
2020-12-26  2:51 ` [PATCH -tip V3 5/8] workqueue: Manually break affinity on hotplug for unbound pool Lai Jiangshan
     [not found]   ` <20201226101631.5448-1-hdanton@sina.com>
2020-12-27 14:04     ` Lai Jiangshan
2020-12-26  2:51 ` [PATCH -tip V3 6/8] workqueue: reorganize workqueue_online_cpu() Lai Jiangshan
2020-12-26  2:51 ` [PATCH -tip V3 7/8] workqueue: reorganize workqueue_offline_cpu() unbind_workers() Lai Jiangshan
2020-12-26  2:51 ` [PATCH -tip V3 8/8] workqueue: Fix affinity of kworkers when attaching into pool Lai Jiangshan
     [not found]   ` <20201229100639.2086-1-hdanton@sina.com>
2020-12-29 10:13     ` Lai Jiangshan
2021-01-08 11:46 ` [PATCH -tip V3 0/8] workqueue: break affinity initiatively Peter Zijlstra
2021-01-11 10:07   ` Thomas Gleixner
2021-01-11 11:01     ` Peter Zijlstra
2021-01-11 15:00       ` Paul E. McKenney
2021-01-11 17:16       ` Peter Zijlstra
2021-01-11 18:09         ` Paul E. McKenney
2021-01-11 21:50           ` Paul E. McKenney
2021-01-12 17:14             ` Paul E. McKenney
2021-01-12 23:53               ` Paul E. McKenney
2021-01-15  9:11                 ` Peter Zijlstra
2021-01-15 13:04                   ` Peter Zijlstra
2021-01-16  6:00                     ` Lai Jiangshan
2021-01-11 19:21         ` Valentin Schneider
2021-01-11 20:23           ` Peter Zijlstra
2021-01-11 22:47             ` Valentin Schneider
2021-01-12  4:33             ` Lai Jiangshan
2021-01-12 14:53               ` Peter Zijlstra
2021-01-12 15:38                 ` Lai Jiangshan
2021-01-13 11:10                   ` Peter Zijlstra
2021-01-13 12:00                     ` Lai Jiangshan
2021-01-13 12:57                     ` Lai Jiangshan
2021-01-12 17:52               ` Valentin Schneider
2021-01-12 14:57           ` Jens Axboe
2021-01-12 15:51             ` Peter Zijlstra

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='CAJhGHyCwyuzikMZAxub=rxn9oe-N2P5C8CEOmyigd9d55SV5YA@mail.gmail.com' \
    --to=jiangshanlai@gmail.com \
    --cc=cai@redhat.com \
    --cc=decui@microsoft.com \
    --cc=laijs@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tj@kernel.org \
    --cc=valentin.schneider@arm.com \
    --cc=vincent.donnefort@arm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.