Hi Jiangshan,
> -----Original Message-----
> From: Lai Jiangshan <jiangshan...@gmail.com>
> Sent: Tuesday, November 3, 2020 12:35 PM
> To: Adrian Huang <adrianhuang0...@gmail.com>
> Cc: Tejun Heo <t...@kernel.org>; LKML <linux-kernel@vger.kernel.org>; Adrian
> Huang12 <ahuan...@lenovo.com>
> Subject: [External] Re: [PATCH 1/1] workqueue: Remove redundant assignment
>
> Hello, Adrian
>
> I believe the pool->node is being used as a node hint before workqueue_init()
> for
> allocating memory. It is useful when it is correct.
Thanks for the comments. I had the same concern in my mind before
submitting this patch. My understanding is that the worker_pool.node
member is used to provide a node hint when allocating the 'worker'
structure or allocating the woker_pool structure (for unbound workqueue
only).
-- Bound workqueue --
The worker structure is allocated when invoking create_worker() in
the end of workqueue_init(), so this won't lead to the potential
problem by removing the pool->node assignment in workqueue_init_early().
-- Unbound workqueue --
The worker_pool structure is allocated in get_unbound_pool().
The function gets the corresponding node id by checking the
global variable 'wq_numa_possible_cpumask' (of course, it depends
on if the global variable 'wq_numa_enabled' is true).
This is not related to 'per_cpu worker_pool.node' (global variable
'cpu_worker_pools').
Please correct me if I miss something. Thanks.
>
> I think it is better to init it early unless there is a bug about it in this
> early stage
> reported (on same archs).
OK, please ignore this patch since the patch is not created for a bug report.
> Thanks
> Lai.-
>
> On Sun, Nov 1, 2020 at 8:21 PM Adrian Huang <adrianhuang0...@gmail.com>
> wrote:
> >
> > From: Adrian Huang <ahuan...@lenovo.com>
> >
> > The member 'node' of worker_pool struct (per_cpu worker_pool) is
> > assigned in workqueue_init_early() and workqueue_init().
> > Commit 2186d9f940b6 ("workqueue: move wq_numa_init() to
> > workqueue_init()") fixes an issue by moving wq_numa_init() to
> > workqueue_init() in order to get the valid 'cpu to node' mapping. So,
> > remove the redundant assignment in workqueue_init_early().
> >
> > Signed-off-by: Adrian Huang <ahuan...@lenovo.com>
> > ---
> > kernel/workqueue.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/kernel/workqueue.c b/kernel/workqueue.c index
> > 437935e7a199..cf8c0df2410e 100644
> > --- a/kernel/workqueue.c
> > +++ b/kernel/workqueue.c
> > @@ -5937,7 +5937,6 @@ void __init workqueue_init_early(void)
> > pool->cpu = cpu;
> > cpumask_copy(pool->attrs->cpumask, cpumask_of(cpu));
> > pool->attrs->nice = std_nice[i++];
> > - pool->node = cpu_to_node(cpu);
> >
> > /* alloc pool ID */
> > mutex_lock(&wq_pool_mutex);
> > --
> > 2.17.1
> >
