From: Adrian Huang <[email protected]>
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 <[email protected]>
---
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
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.
I think it is better to init it early unless there is a bug
about it in this early stage reported (on same archs).
Thanks
Lai.
On Sun, Nov 1, 2020 at 8:21 PM Adrian Huang <[email protected]> wrote:
>
> From: Adrian Huang <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
Hi Jiangshan,
> -----Original Message-----
> From: Lai Jiangshan <[email protected]>
> Sent: Tuesday, November 3, 2020 12:35 PM
> To: Adrian Huang <[email protected]>
> Cc: Tejun Heo <[email protected]>; LKML <[email protected]>; Adrian
> Huang12 <[email protected]>
> 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 <[email protected]>
> wrote:
> >
> > From: Adrian Huang <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
> >