Messages in this thread | | | From | Lai Jiangshan <> | Date | Wed, 13 Jan 2021 20:00:40 +0800 | Subject | Re: [PATCH -tip V3 0/8] workqueue: break affinity initiatively |
| |
On Wed, Jan 13, 2021 at 7:11 PM Peter Zijlstra <peterz@infradead.org> wrote: > > On Tue, Jan 12, 2021 at 11:38:12PM +0800, Lai Jiangshan wrote: > > > But the hard problem is "how to suppress the warning of > > online&!active in __set_cpus_allowed_ptr()" for late spawned > > unbound workers during hotplug. > > I cannot see create_worker() go bad like that. > > The thing is, it uses: > > kthread_bind_mask(, pool->attr->cpumask) > worker_attach_to_pool() > set_cpus_allowed_ptr(, pool->attr->cpumask) > > which means set_cpus_allowed_ptr() must be a NOP, because the affinity > is already set by kthread_bind_mask(). Further, the first wakeup of that > worker will then hit: > > select_task_rq() > is_cpu_allowed() > is_per_cpu_kthread() -- false > select_fallback_rq() > > > So normally that really isn't a problem. I can only see a tiny hole > there, where someone changes the cpumask between kthread_bind_mask() and > set_cpus_allowed_ptr(). AFAICT that can be fixed in two ways: > > - add wq_pool_mutex around things in create_worker(), or > - move the set_cpus_allowed_ptr() out of worker_attach_to_pool() and > into rescuer_thread(). > > Which then brings us to rescuer_thread... If we manage to trigger the > rescuer during hotplug, then yes, I think that can go wobbly.
Oh, I forgot set_cpus_allowed_ptr() is NOP when combined with kthread_bind_mask()(create_worker()).
So the problem becomes "how to suppress the warning of online&!active in __set_cpus_allowed_ptr()" for late *attached unbound rescuer* workers during hotplug.
> > Let me consider that a bit more while I try and make sense of that splat > Paul reported. > > --- > > diff --git a/kernel/workqueue.c b/kernel/workqueue.c > index ec0771e4a3fb..fe05308dc472 100644 > --- a/kernel/workqueue.c > +++ b/kernel/workqueue.c > @@ -1844,15 +1844,19 @@ static struct worker *alloc_worker(int node) > * cpu-[un]hotplugs. > */ > static void worker_attach_to_pool(struct worker *worker, > - struct worker_pool *pool) > + struct worker_pool *pool, > + bool set_affinity) > { > mutex_lock(&wq_pool_attach_mutex); > > - /* > - * set_cpus_allowed_ptr() will fail if the cpumask doesn't have any > - * online CPUs. It'll be re-applied when any of the CPUs come up. > - */ > - set_cpus_allowed_ptr(worker->task, pool->attrs->cpumask); > + if (set_affinity) { > + /* > + * set_cpus_allowed_ptr() will fail if the cpumask doesn't have > + * any online CPUs. It'll be re-applied when any of the CPUs > + * come up. > + */ > + set_cpus_allowed_ptr(worker->task, pool->attrs->cpumask); > + } > > /* > * The wq_pool_attach_mutex ensures %POOL_DISASSOCIATED remains > @@ -1944,7 +1948,7 @@ static struct worker *create_worker(struct worker_pool *pool) > kthread_bind_mask(worker->task, pool->attrs->cpumask); > > /* successful, attach the worker to the pool */ > - worker_attach_to_pool(worker, pool); > + worker_attach_to_pool(worker, pool, false); > > /* start the newly created worker */ > raw_spin_lock_irq(&pool->lock); > @@ -2509,7 +2513,11 @@ static int rescuer_thread(void *__rescuer) > > raw_spin_unlock_irq(&wq_mayday_lock); > > - worker_attach_to_pool(rescuer, pool); > + /* > + * XXX can go splat when running during hot-un-plug and > + * the pool affinity is wobbly. > + */ > + worker_attach_to_pool(rescuer, pool, true); > > raw_spin_lock_irq(&pool->lock); >
| |