workqueue: fix bug when numa mapping is changed

wq_numa_init() will quit directly on some bonkers cases without freeing the memory. Add the missing cleanup code.

Cc Tejun Heo <[email protected]>

Cc Yasuaki Ishimatsu <[email protected]>

Cc "Gu, Zheng" <[email protected]>

Cc tangchen <[email protected]>

Cc Hiroyuki KAMEZAWA <[email protected]>

Signed-off-by Lai Jiangshan <[email protected]>

---
 kernel/workqueue.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

kernel/workqueue.c +0 -0

index 09b685d..a6fd2b8 100644

Quoted 13 lines · Lai Jiangshan

wq_numa_init() will quit directly on some bonkers cases without freeing the memory. Add the missing cleanup code.

Cc Tejun Heo <[email protected]>

Cc Yasuaki Ishimatsu <[email protected]>

Cc "Gu, Zheng" <[email protected]>

Cc tangchen <[email protected]>

Cc Hiroyuki KAMEZAWA <[email protected]>

Signed-off-by Lai Jiangshan <[email protected]>

---
 kernel/workqueue.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

kernel/workqueue.c static void __init wq_numa_init(void) · line 4816

Quoted 11 lines

kernel/workqueue.c +3 -0

index 09b685d..a6fd2b8 100644
@@ -4811,6 +4811,9 @@ static void __init wq_numa_init(void)
 		if (WARN_ON(node == NUMA_NO_NODE)) {
 			pr_warn("workqueue: NUMA node mapping not available for cpu%d, disabling NUMA support\n", cpu);
 			/* happens iff arch is bonkers, let's just proceed */
+			for_each_node(node)
+				free_cpumask_var(tbl[node]);
+			kfree(tbl);

The comment right up there says that this happens if and only if the arch code is seriously broken and it's consciously skipping exception handling. That's a condition where we might as well trigger BUG_ON(). Just leave it alone.

Thanks.

kernel/workqueue.c +27 -0

+static void wq_update_numa_mapping(int cpu)
+{
+	int node, orig_node = NUMA_NO_NODE, new_node = cpu_to_node(cpu);
+
+	lockdep_assert_held(&wq_pool_mutex);
+
+	if (!wq_numa_enabled)
+		return;
+
+	/* the node of onlining CPU is not NUMA_NO_NODE */
+	if (WARN_ON(new_node == NUMA_NO_NODE))
+		return;
+
+	/* test whether the NUMA node mapping is changed. */
+	if (cpumask_test_cpu(cpu, wq_numa_possible_cpumask[new_node]))
+		return;
+
+	/* find the origin node */
+	for_each_node(node) {
+		if (cpumask_test_cpu(cpu, wq_numa_possible_cpumask[node])) {
+			orig_node = node;
+			break;
+		}
+	}
+
+	/* there may be multi mappings changed, re-initial. */
+	cpumask_clear(wq_numa_possible_cpumask[new_node]);

On Fri, Dec 12, 2014 at 06:19:52PM +0800, Lai Jiangshan wrote:
...

Quoted 32 lines

kernel/workqueue.c +32 -0

+static void wq_update_numa_mapping(int cpu)
+{
+	int node, orig_node = NUMA_NO_NODE, new_node = cpu_to_node(cpu);
+
+	lockdep_assert_held(&wq_pool_mutex);
+
+	if (!wq_numa_enabled)
+		return;
+
+	/* the node of onlining CPU is not NUMA_NO_NODE */
+	if (WARN_ON(new_node == NUMA_NO_NODE))
+		return;
+
+	/* test whether the NUMA node mapping is changed. */
+	if (cpumask_test_cpu(cpu, wq_numa_possible_cpumask[new_node]))
+		return;
+
+	/* find the origin node */
+	for_each_node(node) {
+		if (cpumask_test_cpu(cpu, wq_numa_possible_cpumask[node])) {
+			orig_node = node;
+			break;
+		}
+	}
+
+	/* there may be multi mappings changed, re-initial. */
+	cpumask_clear(wq_numa_possible_cpumask[new_node]);
+	if (orig_node != NUMA_NO_NODE)
+		cpumask_clear(wq_numa_possible_cpumask[orig_node]);
+	for_each_possible_cpu(cpu) {
+		node = cpu_to_node(node);
+		if (node == new_node)

kernel/workqueue.c out_unlock: · line 3988

Quoted 5 lines

kernel/workqueue.c +5 -0

+			cpumask_set_cpu(cpu, wq_numa_possible_cpumask[new_node]);
+		else if (orig_node != NUMA_NO_NODE && node == orig_node)
+			cpumask_set_cpu(cpu, wq_numa_possible_cpumask[orig_node]);
+	}
+}

Let's please move this to NUMA code and properly update it on actual mapping changes.

1) "NUMA code" = system's NUMA memory hotplug code, AKA, keep the numa mapping stable

I think this is the better idea. This idea came to my mind immediately at the time I received the bug report. And after some discussions, I was told that it is too HARD to keep the numa mapping stable across multiple physical system-board/node online/offline.

Yasuaki Ishimatsu hit a allocation failure bug when the numa mapping between CPU and node is changed. This was the last scene:
SLUB: Unable to allocate memory on node 2 (gfp=0x80d0)
cache: kmalloc-192, object size: 192, buffer size: 192, default order: 1, min order: 0 node 0: slabs: 6172, objs: 259224, free: 245741
node 1: slabs: 3261, objs: 136962, free: 127656

Yasuaki Ishimatsu investigated that it happened in the following situation:

1) System Node/CPU before offline/online:

	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89
	node 2 | 30-44, 90-104
	node 3 | 45-59, 105-119

2) A system-board (contains node2 and node3) is offline:

	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89

3) A new system-board is online, two new node IDs are allocated for the two node of the SB, but the old CPU IDs are allocated for the SB, here the NUMA mapping between node and CPU is changed. (the node of CPU#30 is changed from node#2 to node#4, for example)

	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89
	node 4 | 30-59
	node 5 | 90-119

4) now, the NUMA mapping is changed, but wq_numa_possible_cpumask which is the convenient NUMA mapping cache in workqueue.c is still outdated. thus pool->node calculated by get_unbound_pool() is incorrect.

5) when the create_worker() is called with the incorrect offlined

    pool->node, it is failed and the pool can't make any progress.

To fix this bug, we need to fixup the wq_numa_possible_cpumask and the pool->node, the fix is so complicated that we split it into two patches, this patch fix the wq_numa_possible_cpumask and the next fix the pool->node.

To fix the wq_numa_possible_cpumask, we only update the cpumasks of the orig_node and the new_node of the onlining @cpu. we con't touch other unrelated nodes since the wq subsystem haven't seen the changed.

After this fix the new pool->node of new pools are correct. and existing wq's affinity is fixed up by wq_update_unbound_numa() after wq_update_numa_mapping().

Reported-by Yasuaki Ishimatsu <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Yasuaki Ishimatsu <[email protected]>

Cc "Gu, Zheng" <[email protected]>

Cc tangchen <[email protected]>

Cc Hiroyuki KAMEZAWA <[email protected]>

Signed-off-by Lai Jiangshan <[email protected]>

---
 kernel/workqueue.c |   42 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 41 insertions(+), 1 deletions(-)

kernel/workqueue.c +22 -1

index a6fd2b8..4c88b61 100644
@@ -266,7 +266,7 @@ struct workqueue_struct {
 static struct kmem_cache *pwq_cache;
 
 static cpumask_var_t *wq_numa_possible_cpumask;
-					/* possible CPUs of each node */
+					/* PL: possible CPUs of each node */
 
 static bool wq_disable_numa;
 module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -3949,6 +3949,44 @@ out_unlock:
 	put_pwq_unlocked(old_pwq);
 }
 
+static void wq_update_numa_mapping(int cpu)
+{
+	int node, orig_node = NUMA_NO_NODE, new_node = cpu_to_node(cpu);
+
+	lockdep_assert_held(&wq_pool_mutex);
+
+	if (!wq_numa_enabled)
+		return;
+
+	/* the node of onlining CPU is not NUMA_NO_NODE */
+	if (WARN_ON(new_node == NUMA_NO_NODE))
+		return;
+
+	/* test whether the NUMA node mapping is changed. */
+	if (cpumask_test_cpu(cpu, wq_numa_possible_cpumask[new_node]))
+		return;
+
+	/* find the origin node */
+	for_each_node(node) {
+		if (cpumask_test_cpu(cpu, wq_numa_possible_cpumask[node])) {
+			orig_node = node;

kernel/workqueue.c +9 -0

+		if (node == new_node)
+			cpumask_set_cpu(cpu, wq_numa_possible_cpumask[new_node]);
+		else if (orig_node != NUMA_NO_NODE && node == orig_node)
+			cpumask_set_cpu(cpu, wq_numa_possible_cpumask[orig_node]);
+	}
+}
+
 static int alloc_and_link_pwqs(struct workqueue_struct *wq)
 {
 	bool highpri = wq->flags & WQ_HIGHPRI;

@@ -4584,6 +4622,8 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
 			mutex_unlock(&pool->attach_mutex);
 		}
 
+		wq_update_numa_mapping(cpu);
+
 		/* update NUMA affinity of unbound workqueues */
 		list_for_each_entry(wq, &workqueues, list)
 			wq_update_unbound_numa(wq, cpu, true);

Yasuaki Ishimatsu hit a bug when the numa mapping between CPU and node is changed. And the previous path fixup wq_numa_possible_cpumask. (See more information form the changelog of that patch)

After wq_numa_possible_cpumask was updated, the new pool->node will be correct, but the existing pools (and workers) are still running, some of them are running with the wrong pool->node, or even worse, with pool->node which is quitted node, they create_worker() on wrong pool->node. These create_worker() may create workers on wrong node or failed without any progress (when with pool->node which is quitted node).

So we need to update the pool->node when the numa mapping is changed.

We simply re-calc the pool->node when the numa mapping changed. It reuses the code from get_unbound_pool() for unbound pool.

Quoted 13 lines · Lai Jiangshan

Yasuaki Ishimatsu hit a bug when the numa mapping between CPU and node is changed. And the previous path fixup wq_numa_possible_cpumask. (See more information form the changelog of that patch)

After wq_numa_possible_cpumask was updated, the new pool->node will be correct, but the existing pools (and workers) are still running, some of them are running with the wrong pool->node, or even worse, with pool->node which is quitted node, they create_worker() on wrong pool->node. These create_worker() may create workers on wrong node or failed without any progress (when with pool->node which is quitted node).

So we need to update the pool->node when the numa mapping is changed.

Quoted 2 lines

We simply re-calc the pool->node when the numa mapping changed. It reuses the code from get_unbound_pool() for unbound pool.

I don't get this patch. If a node is gone, all its cpus would be gone and the pool should be discarded. If a new node comes online with different mappings, new sets of pools should serve them instead of recycling the old ones. Wouldn't it make a lot more sense to make sure we don't reuse the pools w/ old mappings for new pwqs?

Thanks.

The pwqs of the old node's cpumask do be discarded. But the pools of the old node's cpumask maybe recycle. For example, a new workqueue's affinity is set to the old node's cpumask before the pool is dead. Any old pool can long live.

Quoted 3 lines · Lai Jiangshan

The pwqs of the old node's cpumask do be discarded. But the pools of the old node's cpumask maybe recycle. For example, a new workqueue's affinity is set to the old node's cpumask before the pool is dead. Any old pool can long live.

Hah? Why can't it just be unhashed so that it can't be looked up for new pwqs anymore?

Quoted 8 lines · Tejun Heo

Quoted 3 lines · Lai Jiangshan

The pwqs of the old node's cpumask do be discarded. But the pools of the old node's cpumask maybe recycle. For example, a new workqueue's affinity is set to the old node's cpumask before the pool is dead. Any old pool can long live.

Hah? Why can't it just be unhashed so that it can't be looked up for new pwqs anymore?

unhashing doesn't reduce the complexity in my code.

	for_each_pool(pool, pi) {
		node = calc_pool_node(pool);
		if (pool->node != node)

(quoted diff) +1 -1

-			pool->node = node;
+			unhash_pool(pool);

	}

And any old pool can long live due to:
some works queues themself back and back the old pool has extremely long pending works

So I prefer to fixup existing pool->node.

We fixed the major cases when the numa mapping is changed.

We still have the assumption that when the node<->cpu mapping is changed the original node is offline, and the current code of memory-hutplug also prove this.

This assumption might be changed in future and the orig_node is still online in some cases. And in these cases, the cpumask of the pwqs of the orig_node still contains the onlining CPU which is a CPU of another node, and the worker may run on the onlining CPU (aka run on the wrong node).

So we drop this assumption and make the code calls wq_update_unbound_numa() to update the affinity in this case.

Quoted 11 lines · Lai Jiangshan

We fixed the major cases when the numa mapping is changed.

We still have the assumption that when the node<->cpu mapping is changed the original node is offline, and the current code of memory-hutplug also prove this.

This assumption might be changed in future and the orig_node is still online in some cases. And in these cases, the cpumask of the pwqs of the orig_node still contains the onlining CPU which is a CPU of another node, and the worker may run on the onlining CPU (aka run on the wrong node).

Quoted 2 lines

So we drop this assumption and make the code calls wq_update_unbound_numa() to update the affinity in this case.

This is seriously obfuscating. I really don't think meddling with existing pools is a good idea.

The foundation those pools were standing are gone.

Drain and discard the pools. Please don't try to
retro-fit it to new foundations.

Thanks.

Quoted 1 line

The foundation those pools were standing are gone.

This statement is not true unless we write some code to force them, dequeue them from the unbound_pool_hash, for example.

A pwq bound to a specified node might be last long or even forever after the node was offline. Especially when this pwq has some back-to-back work items which requeue themselves and cause the pwq can't quit.

This kinds of pwqs will cause their own pools busy and maybe create workers. This pools will fail on create_worker() since the node is offline.

This case is extremely rare, but it is possible. And we hope create_worker() to be fault-tolerant in this case and other different cases when the node is lack of memory, for example, create_worker() can try to allocate memory from the whole system rather than only the target node, the most important thing is making some progress.

So the solution is that, when the create_worker() fails on a specified node, it will retry with NUMA_NO_NODE for further allocation.

Quoted 13 lines · Lai Jiangshan

A pwq bound to a specified node might be last long or even forever after the node was offline. Especially when this pwq has some back-to-back work items which requeue themselves and cause the pwq can't quit.

This kinds of pwqs will cause their own pools busy and maybe create workers. This pools will fail on create_worker() since the node is offline.

This case is extremely rare, but it is possible. And we hope create_worker() to be fault-tolerant in this case and other different cases when the node is lack of memory, for example, create_worker() can try to allocate memory from the whole system rather than only the target node, the most important thing is making some progress.

Quoted 2 lines

So the solution is that, when the create_worker() fails on a specified node, it will retry with NUMA_NO_NODE for further allocation.

The code looks correct. But I don't think this issue depend on node offlining. The allocation may also fail if node has no memory.

Workqueue code has an assumption that the numa mapping is stable after system booted. It is incorrectly currently.

Yasuaki Ishimatsu hit a allocation failure bug when the numa mapping between CPU and node is changed. This was the last scene:
SLUB: Unable to allocate memory on node 2 (gfp=0x80d0)
cache: kmalloc-192, object size: 192, buffer size: 192, default order: 1, min order: 0 node 0: slabs: 6172, objs: 259224, free: 245741
node 1: slabs: 3261, objs: 136962, free: 127656

Yasuaki Ishimatsu investigated that it happened in the following situation:

1) System Node/CPU before offline/online:

	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89
	node 2 | 30-44, 90-104
	node 3 | 45-59, 105-119

2) A system-board (contains node2 and node3) is offline:

	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89

3) A new system-board is online, two new node IDs are allocated

    for the two node of the SB, but the old CPU IDs are allocated for
    the SB, here the NUMA mapping between node and CPU is changed.
    (the node of CPU#30 is changed from node#2 to node#4, for example)
	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89
	node 4 | 30-59
	node 5 | 90-119

4) now, the NUMA mapping is changed, but wq_numa_possible_cpumask which is the convenient NUMA mapping cache in workqueue.c is still outdated. thus pool->node calculated by get_unbound_pool() is incorrect.

5) when the create_worker() is called with the incorrect offlined

     pool->node, it is failed and the pool can't make any progress.

To fix this bug, we need to fixup the wq_numa_possible_cpumask and the pool->node, it is done in patch2 and patch3.

patch1 fixes memory leak related wq_numa_possible_cpumask. patch4 kill another assumption about how the numa mapping changed. patch5 reduces the allocation fails when the node is offline or the node is lack of memory.

The patchset is untested. It is sent for earlier review.

Thanks,
Lai.

Reported-by Yasuaki Ishimatsu <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Yasuaki Ishimatsu <[email protected]>

Cc "Gu, Zheng" <[email protected]>

Cc tangchen <[email protected]>

Cc Hiroyuki KAMEZAWA <[email protected]>

Lai Jiangshan (5):
workqueue: fix memory leak in wq_numa_init()
workqueue: update wq_numa_possible_cpumask
workqueue: fixup existing pool->node
workqueue: update NUMA affinity for the node lost CPU
workqueue: retry on NUMA_NO_NODE when create_worker() fails

  kernel/workqueue.c |  129 ++++++++++++++++++++++++++++++++++++++++++++--------
  1 files changed, 109 insertions(+), 20 deletions(-)

Hi Lai,

Thank you for posting the patches. I tried your patches. But the following kernel panic occurred.

[  889.394087] BUG: unable to handle kernel paging request at 000000020000f3f1
[  889.395005] IP: [<ffffffff8108fe90>] workqueue_cpu_up_callback+0x510/0x740
[  889.395005] PGD 17a83067 PUD 0
[  889.395005] Oops: 0000 [#1] SMP
[  889.395005] Modules linked in: xt_CHECKSUM ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables
ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_mangle iptable_security iptable_raw iptable_filter ip_tables sg vfat fat x86_pkg_temp_thermal coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel aesni_intel iTCO_wdt sb_edac
iTCO_vendor_support i2c_i801 lrw gf128mul lpc_ich edac_core glue_helper mfd_core ablk_helper cryptd pcspkr shpchp ipmi_devintf ipmi_si ipmi_msghandler tpm_infineon nfsd auth_rpcgss nfs_acl lockd grace sunrpc uinput xfs libcrc32c sd_mod mgag200 syscopyarea sysfillrect sysimgblt drm_kms_helper igb ttm
e1000e lpfc drm dca ptp i2c_algo_bit megaraid_sas scsi_transport_fc pps_core i2c_core dm_mirror dm_region_hash dm_log dm_mod
[  889.395005] CPU: 8 PID: 13595 Comm: udev_dp_bridge. Not tainted 3.18.0Lai+ #26
[  889.395005] Hardware name: FUJITSU PRIMEQUEST2800E/SB, BIOS PRIMEQUEST 2000 Series BIOS Version 01.81 12/03/2014
[  889.395005] task: ffff8a074a145160 ti: ffff8a077a6ec000 task.ti: ffff8a077a6ec000
[  889.395005] RIP: 0010:[<ffffffff8108fe90>]  [<ffffffff8108fe90>] workqueue_cpu_up_callback+0x510/0x740
[  889.395005] RSP: 0018:ffff8a077a6efca8  EFLAGS: 00010202
[  889.395005] RAX: 0000000000000001 RBX: 000000000000edf1 RCX: 000000000000edf1
[  889.395005] RDX: 0000000000000100 RSI: 000000020000f3f1 RDI: 0000000000000001
[  889.395005] RBP: ffff8a077a6efd08 R08: ffffffff81ac6de0 R09: ffff880874610000
[  889.395005] R10: 00000000ffffffff R11: 0000000000000001 R12: 000000000000f3f0
[  889.395005] R13: 000000000000001f R14: 00000000ffffffff R15: ffffffff81ac6de0
[  889.395005] FS:  00007f6b20c67740(0000) GS:ffff88087fd00000(0000) knlGS:0000000000000000
[  889.395005] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  889.395005] CR2: 000000020000f3f1 CR3: 000000004534c000 CR4: 00000000001407e0
[  889.395005] Stack:
[  889.395005]  ffffffffffffffff 0000000000000020 fffffffffffffff8 00000004810a192d
[  889.395005]  ffff8a0700000204 0000000052f5b32d ffffffff81994fc0 00000000fffffff6
[  889.395005]  ffffffff81a13840 0000000000000002 000000000000001f 0000000000000000
[  889.395005] Call Trace:
[  889.395005]  [<ffffffff81094f6c>] notifier_call_chain+0x4c/0x70
[  889.395005]  [<ffffffff8109507e>] __raw_notifier_call_chain+0xe/0x10
[  889.395005]  [<ffffffff810750b3>] cpu_notify+0x23/0x50
[  889.395005]  [<ffffffff81075408>] _cpu_up+0x188/0x1a0
[  889.395005]  [<ffffffff810754a9>] cpu_up+0x89/0xb0
[  889.395005]  [<ffffffff8164f960>] cpu_subsys_online+0x40/0x90
[  889.395005]  [<ffffffff8140f10d>] device_online+0x6d/0xa0
[  889.395005]  [<ffffffff8140f1d5>] online_store+0x95/0xa0
[  889.395005]  [<ffffffff8140c2e8>] dev_attr_store+0x18/0x30
[  889.395005]  [<ffffffff8126210d>] sysfs_kf_write+0x3d/0x50
[  889.395005]  [<ffffffff81261624>] kernfs_fop_write+0xe4/0x160
[  889.395005]  [<ffffffff811e90d7>] vfs_write+0xb7/0x1f0
[  889.395005]  [<ffffffff81021dcc>] ? do_audit_syscall_entry+0x6c/0x70
[  889.395005]  [<ffffffff811e9bc5>] SyS_write+0x55/0xd0
[  889.395005]  [<ffffffff816646a9>] system_call_fastpath+0x12/0x17
[  889.395005] Code: 44 00 00 83 c7 01 48 63 d7 4c 89 ff e8 3a 2a 28 00 8b 15 78 84 a3 00 89 c7 39 d0 7d 70 48 63 cb 4c 89 e6 48 03 34 cd e0 3a ab 81 <8b> 1e 39 5d bc 74 36 41 39 de 74 0c 48 63 f2 eb c7 0f 1f 80 00
[  889.395005] RIP  [<ffffffff8108fe90>] workqueue_cpu_up_callback+0x510/0x740
[  889.395005]  RSP <ffff8a077a6efca8>
[  889.395005] CR2: 000000020000f3f1
[  889.785760] ---[ end trace 39abbfc9f93402f2 ]---
[  889.790931] Kernel panic - not syncing: Fatal exception
[  889.791931] Kernel Offset: 0x0 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffff9fffffff)
[  889.791931] drm_kms_helper: panic occurred, switching back to text console
[  889.815947] ------------[ cut here ]------------
[  889.815947] WARNING: CPU: 8 PID: 64 at arch/x86/kernel/smp.c:124 native_smp_send_reschedule+0x5d/0x60()
[  889.815947] Modules linked in: xt_CHECKSUM ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables
ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_mangle iptable_security iptable_raw iptable_filter ip_tables sg vfat fat x86_pkg_temp_thermal coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel aesni_intel iTCO_wdt sb_edac
iTCO_vendor_support i2c_i801 lrw gf128mul lpc_ich edac_core glue_helper mfd_core ablk_helper cryptd pcspkr shpchp ipmi_devintf ipmi_si ipmi_msghandler tpm_infineon nfsd auth_rpcgss nfs_acl lockd grace sunrpc uinput xfs libcrc32c sd_mod mgag200 syscopyarea sysfillrect sysimgblt drm_kms_helper igb ttm
e1000e lpfc drm dca ptp i2c_algo_bit megaraid_sas scsi_transport_fc pps_core i2c_core dm_mirror dm_region_hash dm_log dm_mod
[  889.815947] CPU: 8 PID: 64 Comm: migration/8 Tainted: G      D        3.18.0Lai+ #26
[  889.815947] Hardware name: FUJITSU PRIMEQUEST2800E/SB, BIOS PRIMEQUEST 2000 Series BIOS Version 01.81 12/03/2014
[  889.815947]  0000000000000000 00000000f7f40529 ffff88087fd03d38 ffffffff8165c8d4
[  889.815947]  0000000000000000 0000000000000000 ffff88087fd03d78 ffffffff81074eb1
[  889.815947]  ffff88087fd03d78 0000000000000000 ffff88087fc13840 0000000000000008
[  889.815947] Call Trace:
[  889.815947]  <IRQ>  [<ffffffff8165c8d4>] dump_stack+0x46/0x58
[  889.815947]  [<ffffffff81074eb1>] warn_slowpath_common+0x81/0xa0
[  889.815947]  [<ffffffff81074fca>] warn_slowpath_null+0x1a/0x20
[  889.815947]  [<ffffffff810489bd>] native_smp_send_reschedule+0x5d/0x60
[  889.815947]  [<ffffffff810b0ad4>] trigger_load_balance+0x144/0x1b0
[  889.815947]  [<ffffffff810a009f>] scheduler_tick+0x9f/0xe0
[  889.815947]  [<ffffffff810daef4>] update_process_times+0x64/0x80
[  889.815947]  [<ffffffff810eab05>] tick_sched_handle.isra.19+0x25/0x60
[  889.815947]  [<ffffffff810eab85>] tick_sched_timer+0x45/0x80
[  889.815947]  [<ffffffff810dbbe7>] __run_hrtimer+0x77/0x1d0
[  889.815947]  [<ffffffff810eab40>] ? tick_sched_handle.isra.19+0x60/0x60
[  889.815947]  [<ffffffff810dbfd7>] hrtimer_interrupt+0xf7/0x240
[  889.815947]  [<ffffffff8104b85b>] local_apic_timer_interrupt+0x3b/0x70
[  889.815947]  [<ffffffff81667465>] smp_apic_timer_interrupt+0x45/0x60
[  889.815947]  [<ffffffff8166553d>] apic_timer_interrupt+0x6d/0x80
[  889.815947]  <EOI>  [<ffffffff810a79c7>] ? set_next_entity+0x67/0x80
[  889.815947]  [<ffffffffa011d1d7>] ? __drm_modeset_lock_all+0x37/0x120 [drm]
[  889.815947]  [<ffffffff8109c727>] ? finish_task_switch+0x57/0x180
[  889.815947]  [<ffffffff8165fba8>] __schedule+0x2e8/0x7e0
[  889.815947]  [<ffffffff816600c9>] schedule+0x29/0x70
[  889.815947]  [<ffffffff81097d43>] smpboot_thread_fn+0xd3/0x1b0
[  889.815947]  [<ffffffff81097c70>] ? SyS_setgroups+0x1a0/0x1a0
[  889.815947]  [<ffffffff81093df1>] kthread+0xe1/0x100
[  889.815947]  [<ffffffff81093d10>] ? kthread_create_on_node+0x1b0/0x1b0
[  889.815947]  [<ffffffff816645fc>] ret_from_fork+0x7c/0xb0
[  889.815947]  [<ffffffff81093d10>] ? kthread_create_on_node+0x1b0/0x1b0
[  889.815947] ---[ end trace 39abbfc9f93402f3 ]---
[  890.156187] ------------[ cut here ]------------
[  890.156187] WARNING: CPU: 8 PID: 64 at arch/x86/kernel/smp.c:124 native_smp_send_reschedule+0x5d/0x60()
[  890.156187] Modules linked in: xt_CHECKSUM ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables
ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_mangle iptable_security iptable_raw iptable_filter ip_tables sg vfat fat x86_pkg_temp_thermal coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel aesni_intel iTCO_wdt sb_edac
iTCO_vendor_support i2c_i801 lrw gf128mul lpc_ich edac_core glue_helper mfd_core ablk_helper cryptd pcspkr shpchp ipmi_devintf ipmi_si ipmi_msghandler tpm_infineon nfsd auth_rpcgss nfs_acl lockd grace sunrpc uinput xfs libcrc32c sd_mod mgag200 syscopyarea sysfillrect sysimgblt drm_kms_helper igb ttm
e1000e lpfc drm dca ptp i2c_algo_bit megaraid_sas scsi_transport_fc pps_core i2c_core dm_mirror dm_region_hash dm_log dm_mod
[  890.156187] CPU: 8 PID: 64 Comm: migration/8 Tainted: G      D W      3.18.0Lai+ #26
[  890.156187] Hardware name: FUJITSU PRIMEQUEST2800E/SB, BIOS PRIMEQUEST 2000 Series BIOS Version 01.81 12/03/2014
[  890.156187]  0000000000000000 00000000f7f40529 ffff88087366bc08 ffffffff8165c8d4
[  890.156187]  0000000000000000 0000000000000000 ffff88087366bc48 ffffffff81074eb1
[  890.156187]  ffff88087fd142c0 0000000000000044 ffff8a074a145160 ffff8a074a145160
[  890.156187] Call Trace:
[  890.156187]  [<ffffffff8165c8d4>] dump_stack+0x46/0x58
[  890.156187]  [<ffffffff81074eb1>] warn_slowpath_common+0x81/0xa0
[  890.156187]  [<ffffffff81074fca>] warn_slowpath_null+0x1a/0x20
[  890.156187]  [<ffffffff810489bd>] native_smp_send_reschedule+0x5d/0x60
[  890.156187]  [<ffffffff8109ddd8>] resched_curr+0xa8/0xd0
[  890.156187]  [<ffffffff8109eac0>] check_preempt_curr+0x80/0xa0
[  890.156187]  [<ffffffff810a78c8>] attach_task+0x48/0x50
[  890.156187]  [<ffffffff810a7ae5>] active_load_balance_cpu_stop+0x105/0x250
[  890.156187]  [<ffffffff810a79e0>] ? set_next_entity+0x80/0x80
[  890.156187]  [<ffffffff8110cab8>] cpu_stopper_thread+0x78/0x150
[  890.156187]  [<ffffffff8165fba8>] ? __schedule+0x2e8/0x7e0
[  890.156187]  [<ffffffff81097d6f>] smpboot_thread_fn+0xff/0x1b0
[  890.156187]  [<ffffffff81097c70>] ? SyS_setgroups+0x1a0/0x1a0
[  890.156187]  [<ffffffff81093df1>] kthread+0xe1/0x100
[  890.156187]  [<ffffffff81093d10>] ? kthread_create_on_node+0x1b0/0x1b0
[  890.156187]  [<ffffffff816645fc>] ret_from_fork+0x7c/0xb0
[  890.156187]  [<ffffffff81093d10>] ? kthread_create_on_node+0x1b0/0x1b0
[  890.156187] ---[ end trace 39abbfc9f93402f4 ]---

Thanks,
Yasuaki Ishimatsu

Quoted 70 lines · Lai Jiangshan

Workqueue code has an assumption that the numa mapping is stable after system booted. It is incorrectly currently.

Yasuaki Ishimatsu hit a allocation failure bug when the numa mapping between CPU and node is changed. This was the last scene:
SLUB: Unable to allocate memory on node 2 (gfp=0x80d0)
cache: kmalloc-192, object size: 192, buffer size: 192, default order: 1, min order: 0 node 0: slabs: 6172, objs: 259224, free: 245741
node 1: slabs: 3261, objs: 136962, free: 127656

Yasuaki Ishimatsu investigated that it happened in the following situation:

1) System Node/CPU before offline/online:

	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89
	node 2 | 30-44, 90-104
	node 3 | 45-59, 105-119

2) A system-board (contains node2 and node3) is offline:

	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89

3) A new system-board is online, two new node IDs are allocated

    for the two node of the SB, but the old CPU IDs are allocated for
    the SB, here the NUMA mapping between node and CPU is changed.
    (the node of CPU#30 is changed from node#2 to node#4, for example)
	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89
	node 4 | 30-59
	node 5 | 90-119

4) now, the NUMA mapping is changed, but wq_numa_possible_cpumask which is the convenient NUMA mapping cache in workqueue.c is still outdated. thus pool->node calculated by get_unbound_pool() is incorrect.

5) when the create_worker() is called with the incorrect offlined

     pool->node, it is failed and the pool can't make any progress.

To fix this bug, we need to fixup the wq_numa_possible_cpumask and the pool->node, it is done in patch2 and patch3.

patch1 fixes memory leak related wq_numa_possible_cpumask. patch4 kill another assumption about how the numa mapping changed. patch5 reduces the allocation fails when the node is offline or the node is lack of memory.

The patchset is untested. It is sent for earlier review.

Thanks,
Lai.

Reported-by Yasuaki Ishimatsu <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Yasuaki Ishimatsu <[email protected]>

Cc "Gu, Zheng" <[email protected]>

Cc tangchen <[email protected]>

Cc Hiroyuki KAMEZAWA <[email protected]>

Lai Jiangshan (5):
workqueue: fix memory leak in wq_numa_init()
workqueue: update wq_numa_possible_cpumask
workqueue: fixup existing pool->node
workqueue: update NUMA affinity for the node lost CPU
workqueue: retry on NUMA_NO_NODE when create_worker() fails

Quoted 3 lines

  kernel/workqueue.c |  129 ++++++++++++++++++++++++++++++++++++++++++++--------
  1 files changed, 109 insertions(+), 20 deletions(-)

.

Thanks for testing. Would you like to use GDB to print the code of "workqueue_cpu_up_callback+0x510" ?

Quoted 4 lines · Yasuaki Ishimatsu

Hi Lai,

Thank you for posting the patches. I tried your patches. But the following kernel panic occurred.

Hi, Yasuaki,

Thanks for testing. Would you like to use GDB to print the code of "workqueue_cpu_up_callback+0x510" ?

Thanks,
Lai

Quoted 201 lines

[  889.394087] BUG: unable to handle kernel paging request at 000000020000f3f1
[  889.395005] IP: [<ffffffff8108fe90>] workqueue_cpu_up_callback+0x510/0x740
[  889.395005] PGD 17a83067 PUD 0
[  889.395005] Oops: 0000 [#1] SMP
[  889.395005] Modules linked in: xt_CHECKSUM ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables
ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_mangle iptable_security iptable_raw iptable_filter ip_tables sg vfat fat x86_pkg_temp_thermal coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel aesni_intel iTCO_wdt sb_edac
iTCO_vendor_support i2c_i801 lrw gf128mul lpc_ich edac_core glue_helper mfd_core ablk_helper cryptd pcspkr shpchp ipmi_devintf ipmi_si ipmi_msghandler tpm_infineon nfsd auth_rpcgss nfs_acl lockd grace sunrpc uinput xfs libcrc32c sd_mod mgag200 syscopyarea sysfillrect sysimgblt drm_kms_helper igb ttm
e1000e lpfc drm dca ptp i2c_algo_bit megaraid_sas scsi_transport_fc pps_core i2c_core dm_mirror dm_region_hash dm_log dm_mod
[  889.395005] CPU: 8 PID: 13595 Comm: udev_dp_bridge. Not tainted 3.18.0Lai+ #26
[  889.395005] Hardware name: FUJITSU PRIMEQUEST2800E/SB, BIOS PRIMEQUEST 2000 Series BIOS Version 01.81 12/03/2014
[  889.395005] task: ffff8a074a145160 ti: ffff8a077a6ec000 task.ti: ffff8a077a6ec000
[  889.395005] RIP: 0010:[<ffffffff8108fe90>]  [<ffffffff8108fe90>] workqueue_cpu_up_callback+0x510/0x740
[  889.395005] RSP: 0018:ffff8a077a6efca8  EFLAGS: 00010202
[  889.395005] RAX: 0000000000000001 RBX: 000000000000edf1 RCX: 000000000000edf1
[  889.395005] RDX: 0000000000000100 RSI: 000000020000f3f1 RDI: 0000000000000001
[  889.395005] RBP: ffff8a077a6efd08 R08: ffffffff81ac6de0 R09: ffff880874610000
[  889.395005] R10: 00000000ffffffff R11: 0000000000000001 R12: 000000000000f3f0
[  889.395005] R13: 000000000000001f R14: 00000000ffffffff R15: ffffffff81ac6de0
[  889.395005] FS:  00007f6b20c67740(0000) GS:ffff88087fd00000(0000) knlGS:0000000000000000
[  889.395005] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  889.395005] CR2: 000000020000f3f1 CR3: 000000004534c000 CR4: 00000000001407e0
[  889.395005] Stack:
[  889.395005]  ffffffffffffffff 0000000000000020 fffffffffffffff8 00000004810a192d
[  889.395005]  ffff8a0700000204 0000000052f5b32d ffffffff81994fc0 00000000fffffff6
[  889.395005]  ffffffff81a13840 0000000000000002 000000000000001f 0000000000000000
[  889.395005] Call Trace:
[  889.395005]  [<ffffffff81094f6c>] notifier_call_chain+0x4c/0x70
[  889.395005]  [<ffffffff8109507e>] __raw_notifier_call_chain+0xe/0x10
[  889.395005]  [<ffffffff810750b3>] cpu_notify+0x23/0x50
[  889.395005]  [<ffffffff81075408>] _cpu_up+0x188/0x1a0
[  889.395005]  [<ffffffff810754a9>] cpu_up+0x89/0xb0
[  889.395005]  [<ffffffff8164f960>] cpu_subsys_online+0x40/0x90
[  889.395005]  [<ffffffff8140f10d>] device_online+0x6d/0xa0
[  889.395005]  [<ffffffff8140f1d5>] online_store+0x95/0xa0
[  889.395005]  [<ffffffff8140c2e8>] dev_attr_store+0x18/0x30
[  889.395005]  [<ffffffff8126210d>] sysfs_kf_write+0x3d/0x50
[  889.395005]  [<ffffffff81261624>] kernfs_fop_write+0xe4/0x160
[  889.395005]  [<ffffffff811e90d7>] vfs_write+0xb7/0x1f0
[  889.395005]  [<ffffffff81021dcc>] ? do_audit_syscall_entry+0x6c/0x70
[  889.395005]  [<ffffffff811e9bc5>] SyS_write+0x55/0xd0
[  889.395005]  [<ffffffff816646a9>] system_call_fastpath+0x12/0x17
[  889.395005] Code: 44 00 00 83 c7 01 48 63 d7 4c 89 ff e8 3a 2a 28 00 8b 15 78 84 a3 00 89 c7 39 d0 7d 70 48 63 cb 4c 89 e6 48 03 34 cd e0 3a ab 81 <8b> 1e 39 5d bc 74 36 41 39 de 74 0c 48 63 f2 eb c7 0f 1f 80 00
[  889.395005] RIP  [<ffffffff8108fe90>] workqueue_cpu_up_callback+0x510/0x740
[  889.395005]  RSP <ffff8a077a6efca8>
[  889.395005] CR2: 000000020000f3f1
[  889.785760] ---[ end trace 39abbfc9f93402f2 ]---
[  889.790931] Kernel panic - not syncing: Fatal exception
[  889.791931] Kernel Offset: 0x0 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffff9fffffff)
[  889.791931] drm_kms_helper: panic occurred, switching back to text console
[  889.815947] ------------[ cut here ]------------
[  889.815947] WARNING: CPU: 8 PID: 64 at arch/x86/kernel/smp.c:124 native_smp_send_reschedule+0x5d/0x60()
[  889.815947] Modules linked in: xt_CHECKSUM ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables
ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_mangle iptable_security iptable_raw iptable_filter ip_tables sg vfat fat x86_pkg_temp_thermal coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel aesni_intel iTCO_wdt sb_edac
iTCO_vendor_support i2c_i801 lrw gf128mul lpc_ich edac_core glue_helper mfd_core ablk_helper cryptd pcspkr shpchp ipmi_devintf ipmi_si ipmi_msghandler tpm_infineon nfsd auth_rpcgss nfs_acl lockd grace sunrpc uinput xfs libcrc32c sd_mod mgag200 syscopyarea sysfillrect sysimgblt drm_kms_helper igb ttm
e1000e lpfc drm dca ptp i2c_algo_bit megaraid_sas scsi_transport_fc pps_core i2c_core dm_mirror dm_region_hash dm_log dm_mod
[  889.815947] CPU: 8 PID: 64 Comm: migration/8 Tainted: G      D        3.18.0Lai+ #26
[  889.815947] Hardware name: FUJITSU PRIMEQUEST2800E/SB, BIOS PRIMEQUEST 2000 Series BIOS Version 01.81 12/03/2014
[  889.815947]  0000000000000000 00000000f7f40529 ffff88087fd03d38 ffffffff8165c8d4
[  889.815947]  0000000000000000 0000000000000000 ffff88087fd03d78 ffffffff81074eb1
[  889.815947]  ffff88087fd03d78 0000000000000000 ffff88087fc13840 0000000000000008
[  889.815947] Call Trace:
[  889.815947]  <IRQ>  [<ffffffff8165c8d4>] dump_stack+0x46/0x58
[  889.815947]  [<ffffffff81074eb1>] warn_slowpath_common+0x81/0xa0
[  889.815947]  [<ffffffff81074fca>] warn_slowpath_null+0x1a/0x20
[  889.815947]  [<ffffffff810489bd>] native_smp_send_reschedule+0x5d/0x60
[  889.815947]  [<ffffffff810b0ad4>] trigger_load_balance+0x144/0x1b0
[  889.815947]  [<ffffffff810a009f>] scheduler_tick+0x9f/0xe0
[  889.815947]  [<ffffffff810daef4>] update_process_times+0x64/0x80
[  889.815947]  [<ffffffff810eab05>] tick_sched_handle.isra.19+0x25/0x60
[  889.815947]  [<ffffffff810eab85>] tick_sched_timer+0x45/0x80
[  889.815947]  [<ffffffff810dbbe7>] __run_hrtimer+0x77/0x1d0
[  889.815947]  [<ffffffff810eab40>] ? tick_sched_handle.isra.19+0x60/0x60
[  889.815947]  [<ffffffff810dbfd7>] hrtimer_interrupt+0xf7/0x240
[  889.815947]  [<ffffffff8104b85b>] local_apic_timer_interrupt+0x3b/0x70
[  889.815947]  [<ffffffff81667465>] smp_apic_timer_interrupt+0x45/0x60
[  889.815947]  [<ffffffff8166553d>] apic_timer_interrupt+0x6d/0x80
[  889.815947]  <EOI>  [<ffffffff810a79c7>] ? set_next_entity+0x67/0x80
[  889.815947]  [<ffffffffa011d1d7>] ? __drm_modeset_lock_all+0x37/0x120 [drm]
[  889.815947]  [<ffffffff8109c727>] ? finish_task_switch+0x57/0x180
[  889.815947]  [<ffffffff8165fba8>] __schedule+0x2e8/0x7e0
[  889.815947]  [<ffffffff816600c9>] schedule+0x29/0x70
[  889.815947]  [<ffffffff81097d43>] smpboot_thread_fn+0xd3/0x1b0
[  889.815947]  [<ffffffff81097c70>] ? SyS_setgroups+0x1a0/0x1a0
[  889.815947]  [<ffffffff81093df1>] kthread+0xe1/0x100
[  889.815947]  [<ffffffff81093d10>] ? kthread_create_on_node+0x1b0/0x1b0
[  889.815947]  [<ffffffff816645fc>] ret_from_fork+0x7c/0xb0
[  889.815947]  [<ffffffff81093d10>] ? kthread_create_on_node+0x1b0/0x1b0
[  889.815947] ---[ end trace 39abbfc9f93402f3 ]---
[  890.156187] ------------[ cut here ]------------
[  890.156187] WARNING: CPU: 8 PID: 64 at arch/x86/kernel/smp.c:124 native_smp_send_reschedule+0x5d/0x60()
[  890.156187] Modules linked in: xt_CHECKSUM ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables
ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_mangle iptable_security iptable_raw iptable_filter ip_tables sg vfat fat x86_pkg_temp_thermal coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel aesni_intel iTCO_wdt sb_edac
iTCO_vendor_support i2c_i801 lrw gf128mul lpc_ich edac_core glue_helper mfd_core ablk_helper cryptd pcspkr shpchp ipmi_devintf ipmi_si ipmi_msghandler tpm_infineon nfsd auth_rpcgss nfs_acl lockd grace sunrpc uinput xfs libcrc32c sd_mod mgag200 syscopyarea sysfillrect sysimgblt drm_kms_helper igb ttm
e1000e lpfc drm dca ptp i2c_algo_bit megaraid_sas scsi_transport_fc pps_core i2c_core dm_mirror dm_region_hash dm_log dm_mod
[  890.156187] CPU: 8 PID: 64 Comm: migration/8 Tainted: G      D W      3.18.0Lai+ #26
[  890.156187] Hardware name: FUJITSU PRIMEQUEST2800E/SB, BIOS PRIMEQUEST 2000 Series BIOS Version 01.81 12/03/2014
[  890.156187]  0000000000000000 00000000f7f40529 ffff88087366bc08 ffffffff8165c8d4
[  890.156187]  0000000000000000 0000000000000000 ffff88087366bc48 ffffffff81074eb1
[  890.156187]  ffff88087fd142c0 0000000000000044 ffff8a074a145160 ffff8a074a145160
[  890.156187] Call Trace:
[  890.156187]  [<ffffffff8165c8d4>] dump_stack+0x46/0x58
[  890.156187]  [<ffffffff81074eb1>] warn_slowpath_common+0x81/0xa0
[  890.156187]  [<ffffffff81074fca>] warn_slowpath_null+0x1a/0x20
[  890.156187]  [<ffffffff810489bd>] native_smp_send_reschedule+0x5d/0x60
[  890.156187]  [<ffffffff8109ddd8>] resched_curr+0xa8/0xd0
[  890.156187]  [<ffffffff8109eac0>] check_preempt_curr+0x80/0xa0
[  890.156187]  [<ffffffff810a78c8>] attach_task+0x48/0x50
[  890.156187]  [<ffffffff810a7ae5>] active_load_balance_cpu_stop+0x105/0x250
[  890.156187]  [<ffffffff810a79e0>] ? set_next_entity+0x80/0x80
[  890.156187]  [<ffffffff8110cab8>] cpu_stopper_thread+0x78/0x150
[  890.156187]  [<ffffffff8165fba8>] ? __schedule+0x2e8/0x7e0
[  890.156187]  [<ffffffff81097d6f>] smpboot_thread_fn+0xff/0x1b0
[  890.156187]  [<ffffffff81097c70>] ? SyS_setgroups+0x1a0/0x1a0
[  890.156187]  [<ffffffff81093df1>] kthread+0xe1/0x100
[  890.156187]  [<ffffffff81093d10>] ? kthread_create_on_node+0x1b0/0x1b0
[  890.156187]  [<ffffffff816645fc>] ret_from_fork+0x7c/0xb0
[  890.156187]  [<ffffffff81093d10>] ? kthread_create_on_node+0x1b0/0x1b0
[  890.156187] ---[ end trace 39abbfc9f93402f4 ]---

Thanks,
Yasuaki Ishimatsu

Quoted 70 lines · Lai Jiangshan

Workqueue code has an assumption that the numa mapping is stable after system booted. It is incorrectly currently.

Yasuaki Ishimatsu hit a allocation failure bug when the numa mapping between CPU and node is changed. This was the last scene:
SLUB: Unable to allocate memory on node 2 (gfp=0x80d0)

    cache: kmalloc-192, object size: 192, buffer size: 192, default order: 1, min order: 0
    node 0: slabs: 6172, objs: 259224, free: 245741
    node 1: slabs: 3261, objs: 136962, free: 127656

Yasuaki Ishimatsu investigated that it happened in the following situation:

1) System Node/CPU before offline/online:

	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89
	node 2 | 30-44, 90-104
	node 3 | 45-59, 105-119

2) A system-board (contains node2 and node3) is offline:

	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89

3) A new system-board is online, two new node IDs are allocated

     for the two node of the SB, but the old CPU IDs are allocated for
     the SB, here the NUMA mapping between node and CPU is changed.
     (the node of CPU#30 is changed from node#2 to node#4, for example)
	       | CPU
	------------------------
	node 0 |  0-14, 60-74
	node 1 | 15-29, 75-89
	node 4 | 30-59
	node 5 | 90-119

4) now, the NUMA mapping is changed, but wq_numa_possible_cpumask which is the convenient NUMA mapping cache in workqueue.c is still outdated. thus pool->node calculated by get_unbound_pool() is incorrect.

5) when the create_worker() is called with the incorrect offlined

      pool->node, it is failed and the pool can't make any progress.

To fix this bug, we need to fixup the wq_numa_possible_cpumask and the pool->node, it is done in patch2 and patch3.

patch1 fixes memory leak related wq_numa_possible_cpumask. patch4 kill another assumption about how the numa mapping changed. patch5 reduces the allocation fails when the node is offline or the node is lack of memory.

The patchset is untested. It is sent for earlier review.

Thanks,
Lai.

Reported-by Yasuaki Ishimatsu <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Yasuaki Ishimatsu <[email protected]>

Cc "Gu, Zheng" <[email protected]>

Cc tangchen <[email protected]>

Cc Hiroyuki KAMEZAWA <[email protected]>

Lai Jiangshan (5):
workqueue: fix memory leak in wq_numa_init()
workqueue: update wq_numa_possible_cpumask
workqueue: fixup existing pool->node
workqueue: update NUMA affinity for the node lost CPU
workqueue: retry on NUMA_NO_NODE when create_worker() fails

Quoted 3 lines

   kernel/workqueue.c |  129 ++++++++++++++++++++++++++++++++++++++++++++--------
   1 files changed, 109 insertions(+), 20 deletions(-)

.

remove node aware unbound pools if node goes offline.

scan unbound workqueue and remove numa affine pool when a node goes offline.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
 kernel/workqueue.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

kernel/workqueue.c +12 -0

index 35f4f00..07b4eb5 100644
@@ -4921,11 +4921,40 @@ early_initcall(init_workqueues);
  * cached pools per cpu should be freed at node unplug
  */
 
+/*
+ * Replace per-node pwq with dfl_pwq because this node disappers.
+ * The new pool will be set at CPU_ONLINE by wq_update_unbound_numa.
+ */
+static void wq_release_unbound_numa(struct workqueue_struct *wq, int nid)
+{
+	struct pool_workqueue *old_pwq = NULL;
+
+	if (!wq_numa_enabled || !(wq->flags & WQ_UNBOUND))
+		return;
+	mutex_lock(&wq->mutex);
+	if (wq->unbound_attrs->no_numa)

kernel/workqueue.c +7 -0

+
 void workqueue_register_numanode(int nid)
 {
 }
 
 void workqueue_unregister_numanode(int nid)
 {
+	struct workqueue_struct *wq;
+
+	mutex_lock(&wq_pool_mutex);
+	list_for_each_entry(wq, &workqueues, list)
+		wq_release_unbound_numa(wq, nid);
+	mutex_unlock(&wq_pool_mutex);
 }
 #endif

Quoted 35 lines · Kamezawa Hiroyuki

remove node aware unbound pools if node goes offline.

scan unbound workqueue and remove numa affine pool when a node goes offline.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
  kernel/workqueue.c | 29 +++++++++++++++++++++++++++++
  1 file changed, 29 insertions(+)

kernel/workqueue.c +17 -0

index 35f4f00..07b4eb5 100644
@@ -4921,11 +4921,40 @@ early_initcall(init_workqueues);
   * cached pools per cpu should be freed at node unplug
   */
 
+/*
+ * Replace per-node pwq with dfl_pwq because this node disappers.
+ * The new pool will be set at CPU_ONLINE by wq_update_unbound_numa.
+ */
+static void wq_release_unbound_numa(struct workqueue_struct *wq, int nid)
+{
+	struct pool_workqueue *old_pwq = NULL;
+
+	if (!wq_numa_enabled || !(wq->flags & WQ_UNBOUND))
+		return;
+	mutex_lock(&wq->mutex);
+	if (wq->unbound_attrs->no_numa)
+		goto out_unlock;
+	spin_lock_irq(&wq->dfl_pwq->pool->lock);
+	get_pwq(wq->dfl_pwq);
+	spin_unlock_irq(&wq->dfl_pwq->pool->lock);
+	old_pwq = numa_pwq_tbl_install(wq, nid, wq->dfl_pwq);

kernel/workqueue.c early_initcall(init_workqueues); · line 4945

Quoted 5 lines

kernel/workqueue.c +5 -0

+out_unlock:
+	mutex_unlock(&wq->mutex);
+	put_pwq_unlocked(old_pwq);
+	return;
+}

We have already did it in wq_update_unbound_numa().

Quoted 15 lines

kernel/workqueue.c +7 -0

+
  void workqueue_register_numanode(int nid)
  {
  }
 
  void workqueue_unregister_numanode(int nid)
  {
+	struct workqueue_struct *wq;
+
+	mutex_lock(&wq_pool_mutex);
+	list_for_each_entry(wq, &workqueues, list)
+		wq_release_unbound_numa(wq, nid);
+	mutex_unlock(&wq_pool_mutex);
  }
  #endif

Although workqueue detects relationship between cpu<->node at boot, it is finally determined in cpu_up().
This patch tries to update pool->node using online status of cpus.

1. When a node goes down, clear per-cpu pool's node attr.
2. When a cpu comes up, update per-cpu pool's node attr.
3. When a cpu comes up, update possinle node cpumask workqueue is using for sched.
4. Detect the best node for unbound pool's cpumask using the latest info.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
 kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 53 insertions(+), 14 deletions(-)

kernel/workqueue.c +29 -14

index 07b4eb5..259b3ba 100644
@@ -266,7 +266,8 @@ struct workqueue_struct {
 static struct kmem_cache *pwq_cache;
 
 static cpumask_var_t *wq_numa_possible_cpumask;
-					/* possible CPUs of each node */
+	/* possible CPUs of each node initialized with possible info at boot.
+           but modified at cpu hotplug to be adjusted to real info.  */
 
 static bool wq_disable_numa;
 module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -3449,6 +3450,31 @@ static void put_unbound_pool(struct worker_pool *pool)
 	call_rcu_sched(&pool->rcu, rcu_free_pool);
 }
 
+/*
+ * detect best node for given cpumask.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+	int node, best, match, selected;
+	static struct cpumask andmask; /* we're under mutex */
+
+	/* Is any node okay ? */
+	if (!wq_numa_enabled ||
+	    cpumask_subset(cpu_online_mask, cpumask))
+		return NUMA_NO_NODE;
+	best = 0;
+	selected = NUMA_NO_NODE;
+	/* select a node which contains the most cpu of cpumask */
+	for_each_node_state(node, N_ONLINE) {
+		cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+		match = cpumask_weight(&andmask);
+		if (match > best)
+			selected = node;
+	}
+	return selected;
+}
+
+
 /**
  * get_unbound_pool - get a worker_pool with the specified attributes
  * @attrs: the attributes of the worker_pool to get

@@ -3467,7 +3493,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
 {
 	u32 hash = wqattrs_hash(attrs);
 	struct worker_pool *pool;
-	int node;
 
 	lockdep_assert_held(&wq_pool_mutex);
 

@@ -3492,17 +3517,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
 	 * 'struct workqueue_attrs' comments for detail.
 	 */
 	pool->attrs->no_numa = false;
-
-	/* if cpumask is contained inside a NUMA node, we belong to that node */
-	if (wq_numa_enabled) {
-		for_each_node(node) {
-			if (cpumask_subset(pool->attrs->cpumask,
-					   wq_numa_possible_cpumask[node])) {
-				pool->node = node;
-				break;
-			}
-		}
-	}
+	pool->node = pool_detect_best_node(pool->attrs->cpumask);
 
 	if (worker_pool_assign_id(pool) < 0)
 		goto fail;

@@ -4567,7 +4582,7 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
 	int cpu = (unsigned long)hcpu;
 	struct worker_pool *pool;
 	struct workqueue_struct *wq;
-	int pi;
+	int pi, node;
 
 	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_UP_PREPARE:

@@ -4583,6 +4598,16 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
 	case CPU_ONLINE:

kernel/workqueue.c +5 -0

+			node = cpu_to_node(cpu);
+			cpumask_set_cpu(cpu, wq_numa_possible_cpumask[node]);
+		}
+		for_each_cpu_worker_pool(pool, cpu)
+			pool->node = cpu_to_node(cpu);

kernel/workqueue.c +14 -0

 		for_each_pool(pool, pi) {
 			mutex_lock(&pool->attach_mutex);

@@ -4951,7 +4976,21 @@ void workqueue_register_numanode(int nid)
 void workqueue_unregister_numanode(int nid)
 {
 	struct workqueue_struct *wq;
+	const struct cpumask *nodecpumask;
+	struct worker_pool *pool;
+	int cpu;
 
+	/* at this point, cpu-to-node relationship is not lost */
+	nodecpumask = cpumask_of_node(nid);
+	for_each_cpu(cpu, nodecpumask) {
+		/*
+		 * pool is allcated at boot and assumed to be persistent,
+		 * we cannot free this.
+		 * Update to be NUMA_NO_NODE. This will be fixed at ONLINE
+		 */
+		for_each_cpu_worker_pool(pool, cpu)
+			pool->node = NUMA_NO_NODE;
+	}
 	mutex_lock(&wq_pool_mutex);
 	list_for_each_entry(wq, &workqueues, list)
 		wq_release_unbound_numa(wq, nid);

Quoted 102 lines · Kamezawa Hiroyuki

Although workqueue detects relationship between cpu<->node at boot, it is finally determined in cpu_up().
This patch tries to update pool->node using online status of cpus.

1. When a node goes down, clear per-cpu pool's node attr.
2. When a cpu comes up, update per-cpu pool's node attr.
3. When a cpu comes up, update possinle node cpumask workqueue is using for sched.
4. Detect the best node for unbound pool's cpumask using the latest info.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
  kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
  1 file changed, 53 insertions(+), 14 deletions(-)

kernel/workqueue.c +31 -14

index 07b4eb5..259b3ba 100644
@@ -266,7 +266,8 @@ struct workqueue_struct {
  static struct kmem_cache *pwq_cache;
 
  static cpumask_var_t *wq_numa_possible_cpumask;
-					/* possible CPUs of each node */
+	/* possible CPUs of each node initialized with possible info at boot.
+           but modified at cpu hotplug to be adjusted to real info.  */
 
  static bool wq_disable_numa;
  module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -3449,6 +3450,31 @@ static void put_unbound_pool(struct worker_pool *pool)
  	call_rcu_sched(&pool->rcu, rcu_free_pool);
  }
 
+/*
+ * detect best node for given cpumask.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+	int node, best, match, selected;
+	static struct cpumask andmask; /* we're under mutex */
+
+	/* Is any node okay ? */
+	if (!wq_numa_enabled ||
+	    cpumask_subset(cpu_online_mask, cpumask))
+		return NUMA_NO_NODE;
+	best = 0;
+	selected = NUMA_NO_NODE;
+	/* select a node which contains the most cpu of cpumask */
+	for_each_node_state(node, N_ONLINE) {
+		cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+		match = cpumask_weight(&andmask);
+		if (match > best)
+			selected = node;
+	}
+	return selected;
+}
+
+
  /**
   * get_unbound_pool - get a worker_pool with the specified attributes
   * @attrs: the attributes of the worker_pool to get

@@ -3467,7 +3493,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
  {
  	u32 hash = wqattrs_hash(attrs);
  	struct worker_pool *pool;
-	int node;
 
  	lockdep_assert_held(&wq_pool_mutex);
 

@@ -3492,17 +3517,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
  	 * 'struct workqueue_attrs' comments for detail.
  	 */
  	pool->attrs->no_numa = false;
-
-	/* if cpumask is contained inside a NUMA node, we belong to that node */
-	if (wq_numa_enabled) {
-		for_each_node(node) {
-			if (cpumask_subset(pool->attrs->cpumask,
-					   wq_numa_possible_cpumask[node])) {
-				pool->node = node;
-				break;
-			}
-		}
-	}
+	pool->node = pool_detect_best_node(pool->attrs->cpumask);
 
  	if (worker_pool_assign_id(pool) < 0)
  		goto fail;

@@ -4567,7 +4582,7 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
  	int cpu = (unsigned long)hcpu;
  	struct worker_pool *pool;
  	struct workqueue_struct *wq;
-	int pi;
+	int pi, node;
 
  	switch (action & ~CPU_TASKS_FROZEN) {
  	case CPU_UP_PREPARE:

@@ -4583,6 +4598,16 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
  	case CPU_ONLINE:
  		mutex_lock(&wq_pool_mutex);
 
+		/* now cpu <-> node info is established, update the info. */
+		if (!wq_disable_numa) {

kernel/workqueue.c static int workqueue_cpu_up_callback(struct notifier_block *nfb, · line 4605

Quoted 3 lines

kernel/workqueue.c +3 -0

+			for_each_node_state(node, N_POSSIBLE)
+				cpumask_clear_cpu(cpu,
+					wq_numa_possible_cpumask[node]);

The wq code try to reuse the origin pwqs/pools when the node still have cpu online. these 3 lines of code will cause the origin pwqs/pools be on the road of dying, and create a new set of pwqs/pools.

Quoted 31 lines

kernel/workqueue.c +19 -0

+			node = cpu_to_node(cpu);
+			cpumask_set_cpu(cpu, wq_numa_possible_cpumask[node]);
+		}
+		for_each_cpu_worker_pool(pool, cpu)
+			pool->node = cpu_to_node(cpu);
  		for_each_pool(pool, pi) {
  			mutex_lock(&pool->attach_mutex);
 

@@ -4951,7 +4976,21 @@ void workqueue_register_numanode(int nid)
  void workqueue_unregister_numanode(int nid)
  {
  	struct workqueue_struct *wq;
+	const struct cpumask *nodecpumask;
+	struct worker_pool *pool;
+	int cpu;
 
+	/* at this point, cpu-to-node relationship is not lost */
+	nodecpumask = cpumask_of_node(nid);
+	for_each_cpu(cpu, nodecpumask) {
+		/*
+		 * pool is allcated at boot and assumed to be persistent,
+		 * we cannot free this.
+		 * Update to be NUMA_NO_NODE. This will be fixed at ONLINE
+		 */
+		for_each_cpu_worker_pool(pool, cpu)
+			pool->node = NUMA_NO_NODE;
+	}
  	mutex_lock(&wq_pool_mutex);
  	list_for_each_entry(wq, &workqueues, list)
  		wq_release_unbound_numa(wq, nid);

Quoted 110 lines · Lai Jiangshan

Quoted 108 lines · Kamezawa Hiroyuki

Although workqueue detects relationship between cpu<->node at boot, it is finally determined in cpu_up().
This patch tries to update pool->node using online status of cpus.

1. When a node goes down, clear per-cpu pool's node attr.
2. When a cpu comes up, update per-cpu pool's node attr.
3. When a cpu comes up, update possinle node cpumask workqueue is using for sched.
4. Detect the best node for unbound pool's cpumask using the latest info.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
  kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
  1 file changed, 53 insertions(+), 14 deletions(-)

kernel/workqueue.c +34 -14

index 07b4eb5..259b3ba 100644
@@ -266,7 +266,8 @@ struct workqueue_struct {
  static struct kmem_cache *pwq_cache;
 
  static cpumask_var_t *wq_numa_possible_cpumask;
-                    /* possible CPUs of each node */
+    /* possible CPUs of each node initialized with possible info at boot.
+           but modified at cpu hotplug to be adjusted to real info.  */
 
  static bool wq_disable_numa;
  module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -3449,6 +3450,31 @@ static void put_unbound_pool(struct worker_pool *pool)
      call_rcu_sched(&pool->rcu, rcu_free_pool);
  }
 
+/*
+ * detect best node for given cpumask.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+    int node, best, match, selected;
+    static struct cpumask andmask; /* we're under mutex */
+
+    /* Is any node okay ? */
+    if (!wq_numa_enabled ||
+        cpumask_subset(cpu_online_mask, cpumask))
+        return NUMA_NO_NODE;
+    best = 0;
+    selected = NUMA_NO_NODE;
+    /* select a node which contains the most cpu of cpumask */
+    for_each_node_state(node, N_ONLINE) {
+        cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+        match = cpumask_weight(&andmask);
+        if (match > best)
+            selected = node;
+    }
+    return selected;
+}
+
+
  /**
   * get_unbound_pool - get a worker_pool with the specified attributes
   * @attrs: the attributes of the worker_pool to get

@@ -3467,7 +3493,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
  {
      u32 hash = wqattrs_hash(attrs);
      struct worker_pool *pool;
-    int node;
 
      lockdep_assert_held(&wq_pool_mutex);
 

@@ -3492,17 +3517,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
       * 'struct workqueue_attrs' comments for detail.
       */
      pool->attrs->no_numa = false;
-
-    /* if cpumask is contained inside a NUMA node, we belong to that node */
-    if (wq_numa_enabled) {
-        for_each_node(node) {
-            if (cpumask_subset(pool->attrs->cpumask,
-                       wq_numa_possible_cpumask[node])) {
-                pool->node = node;
-                break;
-            }
-        }
-    }
+    pool->node = pool_detect_best_node(pool->attrs->cpumask);
 
      if (worker_pool_assign_id(pool) < 0)
          goto fail;

@@ -4567,7 +4582,7 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
      int cpu = (unsigned long)hcpu;
      struct worker_pool *pool;
      struct workqueue_struct *wq;
-    int pi;
+    int pi, node;
 
      switch (action & ~CPU_TASKS_FROZEN) {
      case CPU_UP_PREPARE:

@@ -4583,6 +4598,16 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
      case CPU_ONLINE:
          mutex_lock(&wq_pool_mutex);
 
+        /* now cpu <-> node info is established, update the info. */
+        if (!wq_disable_numa) {
 
 
 
+            for_each_node_state(node, N_POSSIBLE)
+                cpumask_clear_cpu(cpu,
+                    wq_numa_possible_cpumask[node]);

Quoted 3 lines

The wq code try to reuse the origin pwqs/pools when the node still have cpu online. these 3 lines of code will cause the origin pwqs/pools be on the road of dying, and create a new set of pwqs/pools.

because the result of wq_calc_node_cpumask() changes ?

Do you mean some comment should be added here ? or explaination for your reply for [3/4] ?

Thanks,
-Kame

Quoted 17 lines

Quoted 17 lines

kernel/workqueue.c +5 -0

+            node = cpu_to_node(cpu);
+            cpumask_set_cpu(cpu, wq_numa_possible_cpumask[node]);
+        }
+        for_each_cpu_worker_pool(pool, cpu)
+            pool->node = cpu_to_node(cpu);

          for_each_pool(pool, pi) {
              mutex_lock(&pool->attach_mutex);

kernel/workqueue.c +3 -0

@@ -4951,7 +4976,21 @@ void workqueue_register_numanode(int nid)
  void workqueue_unregister_numanode(int nid)
  {
      struct workqueue_struct *wq;
+    const struct cpumask *nodecpumask;
+    struct worker_pool *pool;
+    int cpu;
 

Quoted 15 lines

Quoted 4 lines

kernel/workqueue.c +4 -0

+    /* at this point, cpu-to-node relationship is not lost */
+    nodecpumask = cpumask_of_node(nid);
+    for_each_cpu(cpu, nodecpumask) {
+        /*

kernel/workqueue.c void workqueue_register_numanode(int nid) · line 4996

Quoted 10 lines

kernel/workqueue.c +7 -0

+         * pool is allcated at boot and assumed to be persistent,
+         * we cannot free this.
+         * Update to be NUMA_NO_NODE. This will be fixed at ONLINE
+         */
+        for_each_cpu_worker_pool(pool, cpu)
+            pool->node = NUMA_NO_NODE;
+    }
      mutex_lock(&wq_pool_mutex);
      list_for_each_entry(wq, &workqueues, list)
          wq_release_unbound_numa(wq, nid);

.

Quoted 115 lines · Kamezawa Hiroyuki

Quoted 113 lines · Lai Jiangshan

Quoted 98 lines · Kamezawa Hiroyuki

Although workqueue detects relationship between cpu<->node at boot, it is finally determined in cpu_up().
This patch tries to update pool->node using online status of cpus.

1. When a node goes down, clear per-cpu pool's node attr.
2. When a cpu comes up, update per-cpu pool's node attr.
3. When a cpu comes up, update possinle node cpumask workqueue is using for sched.
4. Detect the best node for unbound pool's cpumask using the latest info.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
   kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
   1 file changed, 53 insertions(+), 14 deletions(-)

kernel/workqueue.c +29 -14

index 07b4eb5..259b3ba 100644
@@ -266,7 +266,8 @@ struct workqueue_struct {
   static struct kmem_cache *pwq_cache;
 
   static cpumask_var_t *wq_numa_possible_cpumask;
-                    /* possible CPUs of each node */
+    /* possible CPUs of each node initialized with possible info at boot.
+           but modified at cpu hotplug to be adjusted to real info.  */
 
   static bool wq_disable_numa;
   module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -3449,6 +3450,31 @@ static void put_unbound_pool(struct worker_pool *pool)
       call_rcu_sched(&pool->rcu, rcu_free_pool);
   }
 
+/*
+ * detect best node for given cpumask.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+    int node, best, match, selected;
+    static struct cpumask andmask; /* we're under mutex */
+
+    /* Is any node okay ? */
+    if (!wq_numa_enabled ||
+        cpumask_subset(cpu_online_mask, cpumask))
+        return NUMA_NO_NODE;
+    best = 0;
+    selected = NUMA_NO_NODE;
+    /* select a node which contains the most cpu of cpumask */
+    for_each_node_state(node, N_ONLINE) {
+        cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+        match = cpumask_weight(&andmask);
+        if (match > best)
+            selected = node;
+    }
+    return selected;
+}
+
+
   /**
    * get_unbound_pool - get a worker_pool with the specified attributes
    * @attrs: the attributes of the worker_pool to get

@@ -3467,7 +3493,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
   {
       u32 hash = wqattrs_hash(attrs);
       struct worker_pool *pool;
-    int node;
 
       lockdep_assert_held(&wq_pool_mutex);
 

@@ -3492,17 +3517,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
        * 'struct workqueue_attrs' comments for detail.
        */
       pool->attrs->no_numa = false;
-
-    /* if cpumask is contained inside a NUMA node, we belong to that node */
-    if (wq_numa_enabled) {
-        for_each_node(node) {
-            if (cpumask_subset(pool->attrs->cpumask,
-                       wq_numa_possible_cpumask[node])) {
-                pool->node = node;
-                break;
-            }
-        }
-    }
+    pool->node = pool_detect_best_node(pool->attrs->cpumask);
 
       if (worker_pool_assign_id(pool) < 0)
           goto fail;

@@ -4567,7 +4582,7 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
       int cpu = (unsigned long)hcpu;
       struct worker_pool *pool;
       struct workqueue_struct *wq;
-    int pi;
+    int pi, node;
 
       switch (action & ~CPU_TASKS_FROZEN) {
       case CPU_UP_PREPARE:

@@ -4583,6 +4598,16 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
       case CPU_ONLINE:

kernel/workqueue.c static int workqueue_cpu_up_callback(struct notifier_block *nfb, · line 4608

Quoted 10 lines

kernel/workqueue.c +5 -0

           mutex_lock(&wq_pool_mutex);
 
+        /* now cpu <-> node info is established, update the info. */
+        if (!wq_disable_numa) {
 
 
 
+            for_each_node_state(node, N_POSSIBLE)
+                cpumask_clear_cpu(cpu,
+                    wq_numa_possible_cpumask[node]);

The wq code try to reuse the origin pwqs/pools when the node still have cpu online. these 3 lines of code will cause the origin pwqs/pools be on the road of dying, and create a new set of pwqs/pools.

Quoted 1 line

because the result of wq_calc_node_cpumask() changes ?

Yes.

Quoted 1 line

Do you mean some comment should be added here ? or explaination for your reply for [3/4] ?

this fix [4/4] breaks the original design.

Quoted 122 lines · Lai Jiangshan

Quoted 111 lines · Kamezawa Hiroyuki

Quoted 110 lines · Lai Jiangshan

Quoted 108 lines · Kamezawa Hiroyuki

Although workqueue detects relationship between cpu<->node at boot, it is finally determined in cpu_up().
This patch tries to update pool->node using online status of cpus.

1. When a node goes down, clear per-cpu pool's node attr.
2. When a cpu comes up, update per-cpu pool's node attr.
3. When a cpu comes up, update possinle node cpumask workqueue is using for sched.
4. Detect the best node for unbound pool's cpumask using the latest info.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
   kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
   1 file changed, 53 insertions(+), 14 deletions(-)

kernel/workqueue.c +34 -14

index 07b4eb5..259b3ba 100644
@@ -266,7 +266,8 @@ struct workqueue_struct {
   static struct kmem_cache *pwq_cache;
 
   static cpumask_var_t *wq_numa_possible_cpumask;
-                    /* possible CPUs of each node */
+    /* possible CPUs of each node initialized with possible info at boot.
+           but modified at cpu hotplug to be adjusted to real info.  */
 
   static bool wq_disable_numa;
   module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -3449,6 +3450,31 @@ static void put_unbound_pool(struct worker_pool *pool)
       call_rcu_sched(&pool->rcu, rcu_free_pool);
   }
 
+/*
+ * detect best node for given cpumask.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+    int node, best, match, selected;
+    static struct cpumask andmask; /* we're under mutex */
+
+    /* Is any node okay ? */
+    if (!wq_numa_enabled ||
+        cpumask_subset(cpu_online_mask, cpumask))
+        return NUMA_NO_NODE;
+    best = 0;
+    selected = NUMA_NO_NODE;
+    /* select a node which contains the most cpu of cpumask */
+    for_each_node_state(node, N_ONLINE) {
+        cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+        match = cpumask_weight(&andmask);
+        if (match > best)
+            selected = node;
+    }
+    return selected;
+}
+
+
   /**
    * get_unbound_pool - get a worker_pool with the specified attributes
    * @attrs: the attributes of the worker_pool to get

@@ -3467,7 +3493,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
   {
       u32 hash = wqattrs_hash(attrs);
       struct worker_pool *pool;
-    int node;
 
       lockdep_assert_held(&wq_pool_mutex);
 

@@ -3492,17 +3517,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
        * 'struct workqueue_attrs' comments for detail.
        */
       pool->attrs->no_numa = false;
-
-    /* if cpumask is contained inside a NUMA node, we belong to that node */
-    if (wq_numa_enabled) {
-        for_each_node(node) {
-            if (cpumask_subset(pool->attrs->cpumask,
-                       wq_numa_possible_cpumask[node])) {
-                pool->node = node;
-                break;
-            }
-        }
-    }
+    pool->node = pool_detect_best_node(pool->attrs->cpumask);
 
       if (worker_pool_assign_id(pool) < 0)
           goto fail;

@@ -4567,7 +4582,7 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
       int cpu = (unsigned long)hcpu;
       struct worker_pool *pool;
       struct workqueue_struct *wq;
-    int pi;
+    int pi, node;
 
       switch (action & ~CPU_TASKS_FROZEN) {
       case CPU_UP_PREPARE:

@@ -4583,6 +4598,16 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
       case CPU_ONLINE:
           mutex_lock(&wq_pool_mutex);
 
+        /* now cpu <-> node info is established, update the info. */
+        if (!wq_disable_numa) {
 
 
 
+            for_each_node_state(node, N_POSSIBLE)
+                cpumask_clear_cpu(cpu,
+                    wq_numa_possible_cpumask[node]);

Quoted 5 lines

Quoted 3 lines

The wq code try to reuse the origin pwqs/pools when the node still have cpu online. these 3 lines of code will cause the origin pwqs/pools be on the road of dying, and create a new set of pwqs/pools.

because the result of wq_calc_node_cpumask() changes ?

Yes.

Quoted 1 line

Do you mean some comment should be added here ? or explaination for your reply for [3/4] ?

Quoted 2 lines

this fix [4/4] breaks the original design.

I'm sorry that I can't understand what this patch breaks. Do you mean it's better to work with broken wq_numa_possible_cpumask ?

I guess removing wq_numa_possible_mask entirely may be the best way byusing online_mask of the node.

Thanks,
-Kame

.

Quoted 118 lines · Lai Jiangshan

Quoted 116 lines · Kamezawa Hiroyuki

Quoted 106 lines · Lai Jiangshan

Quoted 102 lines · Kamezawa Hiroyuki

Although workqueue detects relationship between cpu<->node at boot, it is finally determined in cpu_up().
This patch tries to update pool->node using online status of cpus.

1. When a node goes down, clear per-cpu pool's node attr.
2. When a cpu comes up, update per-cpu pool's node attr.
3. When a cpu comes up, update possinle node cpumask workqueue is using for sched.
4. Detect the best node for unbound pool's cpumask using the latest info.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
   kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
   1 file changed, 53 insertions(+), 14 deletions(-)

kernel/workqueue.c +31 -14

index 07b4eb5..259b3ba 100644
@@ -266,7 +266,8 @@ struct workqueue_struct {
   static struct kmem_cache *pwq_cache;
 
   static cpumask_var_t *wq_numa_possible_cpumask;
-                    /* possible CPUs of each node */
+    /* possible CPUs of each node initialized with possible info at boot.
+           but modified at cpu hotplug to be adjusted to real info.  */
 
   static bool wq_disable_numa;
   module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -3449,6 +3450,31 @@ static void put_unbound_pool(struct worker_pool *pool)
       call_rcu_sched(&pool->rcu, rcu_free_pool);
   }
 
+/*
+ * detect best node for given cpumask.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+    int node, best, match, selected;
+    static struct cpumask andmask; /* we're under mutex */
+
+    /* Is any node okay ? */
+    if (!wq_numa_enabled ||
+        cpumask_subset(cpu_online_mask, cpumask))
+        return NUMA_NO_NODE;
+    best = 0;
+    selected = NUMA_NO_NODE;
+    /* select a node which contains the most cpu of cpumask */
+    for_each_node_state(node, N_ONLINE) {
+        cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+        match = cpumask_weight(&andmask);
+        if (match > best)
+            selected = node;
+    }
+    return selected;
+}
+
+
   /**
    * get_unbound_pool - get a worker_pool with the specified attributes
    * @attrs: the attributes of the worker_pool to get

@@ -3467,7 +3493,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
   {
       u32 hash = wqattrs_hash(attrs);
       struct worker_pool *pool;
-    int node;
 
       lockdep_assert_held(&wq_pool_mutex);
 

@@ -3492,17 +3517,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
        * 'struct workqueue_attrs' comments for detail.
        */
       pool->attrs->no_numa = false;
-
-    /* if cpumask is contained inside a NUMA node, we belong to that node */
-    if (wq_numa_enabled) {
-        for_each_node(node) {
-            if (cpumask_subset(pool->attrs->cpumask,
-                       wq_numa_possible_cpumask[node])) {
-                pool->node = node;
-                break;
-            }
-        }
-    }
+    pool->node = pool_detect_best_node(pool->attrs->cpumask);
 
       if (worker_pool_assign_id(pool) < 0)
           goto fail;

@@ -4567,7 +4582,7 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
       int cpu = (unsigned long)hcpu;
       struct worker_pool *pool;
       struct workqueue_struct *wq;
-    int pi;
+    int pi, node;
 
       switch (action & ~CPU_TASKS_FROZEN) {
       case CPU_UP_PREPARE:

@@ -4583,6 +4598,16 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
       case CPU_ONLINE:
           mutex_lock(&wq_pool_mutex);
 
+        /* now cpu <-> node info is established, update the info. */
+        if (!wq_disable_numa) {

Quoted 7 lines

kernel/workqueue.c static int workqueue_cpu_up_callback(struct notifier_block *nfb, · line 4605

Quoted 3 lines

kernel/workqueue.c +3 -0

+            for_each_node_state(node, N_POSSIBLE)
+                cpumask_clear_cpu(cpu,
+                    wq_numa_possible_cpumask[node]);

The wq code try to reuse the origin pwqs/pools when the node still have cpu online. these 3 lines of code will cause the origin pwqs/pools be on the road of dying, and create a new set of pwqs/pools.

because the result of wq_calc_node_cpumask() changes ?

Quoted 6 lines

Yes.

Quoted 1 line

Do you mean some comment should be added here ? or explaination for your reply for [3/4] ?

this fix [4/4] breaks the original design.

I'm sorry that I can't understand what this patch breaks.

Quoted 121 lines · Kamezawa Hiroyuki

Quoted 120 lines · Lai Jiangshan

Quoted 111 lines · Kamezawa Hiroyuki

Quoted 110 lines · Lai Jiangshan

Quoted 108 lines · Kamezawa Hiroyuki

Although workqueue detects relationship between cpu<->node at boot, it is finally determined in cpu_up().
This patch tries to update pool->node using online status of cpus.

1. When a node goes down, clear per-cpu pool's node attr.
2. When a cpu comes up, update per-cpu pool's node attr.
3. When a cpu comes up, update possinle node cpumask workqueue is using for sched.
4. Detect the best node for unbound pool's cpumask using the latest info.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
    kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
    1 file changed, 53 insertions(+), 14 deletions(-)

kernel/workqueue.c +34 -14

index 07b4eb5..259b3ba 100644
@@ -266,7 +266,8 @@ struct workqueue_struct {
    static struct kmem_cache *pwq_cache;
 
    static cpumask_var_t *wq_numa_possible_cpumask;
-                    /* possible CPUs of each node */
+    /* possible CPUs of each node initialized with possible info at boot.
+           but modified at cpu hotplug to be adjusted to real info.  */
 
    static bool wq_disable_numa;
    module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -3449,6 +3450,31 @@ static void put_unbound_pool(struct worker_pool *pool)
        call_rcu_sched(&pool->rcu, rcu_free_pool);
    }
 
+/*
+ * detect best node for given cpumask.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+    int node, best, match, selected;
+    static struct cpumask andmask; /* we're under mutex */
+
+    /* Is any node okay ? */
+    if (!wq_numa_enabled ||
+        cpumask_subset(cpu_online_mask, cpumask))
+        return NUMA_NO_NODE;
+    best = 0;
+    selected = NUMA_NO_NODE;
+    /* select a node which contains the most cpu of cpumask */
+    for_each_node_state(node, N_ONLINE) {
+        cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+        match = cpumask_weight(&andmask);
+        if (match > best)
+            selected = node;
+    }
+    return selected;
+}
+
+
    /**
     * get_unbound_pool - get a worker_pool with the specified attributes
     * @attrs: the attributes of the worker_pool to get

@@ -3467,7 +3493,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
    {
        u32 hash = wqattrs_hash(attrs);
        struct worker_pool *pool;
-    int node;
 
        lockdep_assert_held(&wq_pool_mutex);
 

@@ -3492,17 +3517,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
         * 'struct workqueue_attrs' comments for detail.
         */
        pool->attrs->no_numa = false;
-
-    /* if cpumask is contained inside a NUMA node, we belong to that node */
-    if (wq_numa_enabled) {
-        for_each_node(node) {
-            if (cpumask_subset(pool->attrs->cpumask,
-                       wq_numa_possible_cpumask[node])) {
-                pool->node = node;
-                break;
-            }
-        }
-    }
+    pool->node = pool_detect_best_node(pool->attrs->cpumask);
 
        if (worker_pool_assign_id(pool) < 0)
            goto fail;

@@ -4567,7 +4582,7 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
        int cpu = (unsigned long)hcpu;
        struct worker_pool *pool;
        struct workqueue_struct *wq;
-    int pi;
+    int pi, node;
 
        switch (action & ~CPU_TASKS_FROZEN) {
        case CPU_UP_PREPARE:

@@ -4583,6 +4598,16 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
        case CPU_ONLINE:
            mutex_lock(&wq_pool_mutex);
 
+        /* now cpu <-> node info is established, update the info. */
+        if (!wq_disable_numa) {
 
 
 
+            for_each_node_state(node, N_POSSIBLE)
+                cpumask_clear_cpu(cpu,
+                    wq_numa_possible_cpumask[node]);

Quoted 5 lines

Quoted 3 lines

The wq code try to reuse the origin pwqs/pools when the node still have cpu online. these 3 lines of code will cause the origin pwqs/pools be on the road of dying, and create a new set of pwqs/pools.

because the result of wq_calc_node_cpumask() changes ?

Yes.

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

Do you mean some comment should be added here ? or explaination for your reply for [3/4] ?

this fix [4/4] breaks the original design.

I'm sorry that I can't understand what this patch breaks.

the pwqs/pools should be kept if the node still have cpu online.

Quoted 122 lines · Lai Jiangshan

Quoted 121 lines · Kamezawa Hiroyuki

Quoted 120 lines · Lai Jiangshan

Quoted 111 lines · Kamezawa Hiroyuki

Quoted 110 lines · Lai Jiangshan

Quoted 108 lines · Kamezawa Hiroyuki

Although workqueue detects relationship between cpu<->node at boot, it is finally determined in cpu_up().
This patch tries to update pool->node using online status of cpus.

1. When a node goes down, clear per-cpu pool's node attr.
2. When a cpu comes up, update per-cpu pool's node attr.
3. When a cpu comes up, update possinle node cpumask workqueue is using for sched.
4. Detect the best node for unbound pool's cpumask using the latest info.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
    kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
    1 file changed, 53 insertions(+), 14 deletions(-)

kernel/workqueue.c +34 -14

index 07b4eb5..259b3ba 100644
@@ -266,7 +266,8 @@ struct workqueue_struct {
    static struct kmem_cache *pwq_cache;
 
    static cpumask_var_t *wq_numa_possible_cpumask;
-                    /* possible CPUs of each node */
+    /* possible CPUs of each node initialized with possible info at boot.
+           but modified at cpu hotplug to be adjusted to real info.  */
 
    static bool wq_disable_numa;
    module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -3449,6 +3450,31 @@ static void put_unbound_pool(struct worker_pool *pool)
        call_rcu_sched(&pool->rcu, rcu_free_pool);
    }
 
+/*
+ * detect best node for given cpumask.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+    int node, best, match, selected;
+    static struct cpumask andmask; /* we're under mutex */
+
+    /* Is any node okay ? */
+    if (!wq_numa_enabled ||
+        cpumask_subset(cpu_online_mask, cpumask))
+        return NUMA_NO_NODE;
+    best = 0;
+    selected = NUMA_NO_NODE;
+    /* select a node which contains the most cpu of cpumask */
+    for_each_node_state(node, N_ONLINE) {
+        cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+        match = cpumask_weight(&andmask);
+        if (match > best)
+            selected = node;
+    }
+    return selected;
+}
+
+
    /**
     * get_unbound_pool - get a worker_pool with the specified attributes
     * @attrs: the attributes of the worker_pool to get

@@ -3467,7 +3493,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
    {
        u32 hash = wqattrs_hash(attrs);
        struct worker_pool *pool;
-    int node;
 
        lockdep_assert_held(&wq_pool_mutex);
 

@@ -3492,17 +3517,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
         * 'struct workqueue_attrs' comments for detail.
         */
        pool->attrs->no_numa = false;
-
-    /* if cpumask is contained inside a NUMA node, we belong to that node */
-    if (wq_numa_enabled) {
-        for_each_node(node) {
-            if (cpumask_subset(pool->attrs->cpumask,
-                       wq_numa_possible_cpumask[node])) {
-                pool->node = node;
-                break;
-            }
-        }
-    }
+    pool->node = pool_detect_best_node(pool->attrs->cpumask);
 
        if (worker_pool_assign_id(pool) < 0)
            goto fail;

@@ -4567,7 +4582,7 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
        int cpu = (unsigned long)hcpu;
        struct worker_pool *pool;
        struct workqueue_struct *wq;
-    int pi;
+    int pi, node;
 
        switch (action & ~CPU_TASKS_FROZEN) {
        case CPU_UP_PREPARE:

@@ -4583,6 +4598,16 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
        case CPU_ONLINE:
            mutex_lock(&wq_pool_mutex);
 
+        /* now cpu <-> node info is established, update the info. */
+        if (!wq_disable_numa) {
 
 
 
+            for_each_node_state(node, N_POSSIBLE)
+                cpumask_clear_cpu(cpu,
+                    wq_numa_possible_cpumask[node]);

Quoted 5 lines

Quoted 3 lines

The wq code try to reuse the origin pwqs/pools when the node still have cpu online. these 3 lines of code will cause the origin pwqs/pools be on the road of dying, and create a new set of pwqs/pools.

because the result of wq_calc_node_cpumask() changes ?

Yes.

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

Do you mean some comment should be added here ? or explaination for your reply for [3/4] ?

this fix [4/4] breaks the original design.

I'm sorry that I can't understand what this patch breaks.

the pwqs/pools should be kept if the node still have cpu online.

So, the fix's grand design should be

1. drop old pwq/pools only at node offline.
2. set proper pool->node based on online node info.
3. update pool->node of per-cpu-pool at cpu ONLINE.

Hm. (1) is done because cpumask_of_node() turns to be zero-filled after all cpus on a node offlined.

But, cpu-to-node relationship cannot be available until a cpu get onlined. It changes at every cpu onlining. So, at node online, refleshing cpumasks

of workqueues only after _all_ cpus on node are onlined seems to be the only way

but I'm not sure how to get a mask of possible cpus on a node.

Possible another way may be using auto numa balancing's numa node hint for worker scheduling.

Do you have any idea ?

Thanks,
-Kame

.

Quoted 138 lines · Kamezawa Hiroyuki

Quoted 122 lines · Lai Jiangshan

Quoted 121 lines · Kamezawa Hiroyuki

Quoted 120 lines · Lai Jiangshan

Quoted 111 lines · Kamezawa Hiroyuki

Quoted 110 lines · Lai Jiangshan

Quoted 108 lines · Kamezawa Hiroyuki

Although workqueue detects relationship between cpu<->node at boot, it is finally determined in cpu_up().
This patch tries to update pool->node using online status of cpus.

1. When a node goes down, clear per-cpu pool's node attr.
2. When a cpu comes up, update per-cpu pool's node attr.
3. When a cpu comes up, update possinle node cpumask workqueue is using for sched.
4. Detect the best node for unbound pool's cpumask using the latest info.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
     kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
     1 file changed, 53 insertions(+), 14 deletions(-)

kernel/workqueue.c +34 -14

index 07b4eb5..259b3ba 100644
@@ -266,7 +266,8 @@ struct workqueue_struct {
     static struct kmem_cache *pwq_cache;
 
     static cpumask_var_t *wq_numa_possible_cpumask;
-                    /* possible CPUs of each node */
+    /* possible CPUs of each node initialized with possible info at boot.
+           but modified at cpu hotplug to be adjusted to real info.  */
 
     static bool wq_disable_numa;
     module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -3449,6 +3450,31 @@ static void put_unbound_pool(struct worker_pool *pool)
         call_rcu_sched(&pool->rcu, rcu_free_pool);
     }
 
+/*
+ * detect best node for given cpumask.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+    int node, best, match, selected;
+    static struct cpumask andmask; /* we're under mutex */
+
+    /* Is any node okay ? */
+    if (!wq_numa_enabled ||
+        cpumask_subset(cpu_online_mask, cpumask))
+        return NUMA_NO_NODE;
+    best = 0;
+    selected = NUMA_NO_NODE;
+    /* select a node which contains the most cpu of cpumask */
+    for_each_node_state(node, N_ONLINE) {
+        cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+        match = cpumask_weight(&andmask);
+        if (match > best)
+            selected = node;
+    }
+    return selected;
+}
+
+
     /**
      * get_unbound_pool - get a worker_pool with the specified attributes
      * @attrs: the attributes of the worker_pool to get

@@ -3467,7 +3493,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
     {
         u32 hash = wqattrs_hash(attrs);
         struct worker_pool *pool;
-    int node;
 
         lockdep_assert_held(&wq_pool_mutex);
 

@@ -3492,17 +3517,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
          * 'struct workqueue_attrs' comments for detail.
          */
         pool->attrs->no_numa = false;
-
-    /* if cpumask is contained inside a NUMA node, we belong to that node */
-    if (wq_numa_enabled) {
-        for_each_node(node) {
-            if (cpumask_subset(pool->attrs->cpumask,
-                       wq_numa_possible_cpumask[node])) {
-                pool->node = node;
-                break;
-            }
-        }
-    }
+    pool->node = pool_detect_best_node(pool->attrs->cpumask);
 
         if (worker_pool_assign_id(pool) < 0)
             goto fail;

@@ -4567,7 +4582,7 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
         int cpu = (unsigned long)hcpu;
         struct worker_pool *pool;
         struct workqueue_struct *wq;
-    int pi;
+    int pi, node;
 
         switch (action & ~CPU_TASKS_FROZEN) {
         case CPU_UP_PREPARE:

@@ -4583,6 +4598,16 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
         case CPU_ONLINE:
             mutex_lock(&wq_pool_mutex);
 
+        /* now cpu <-> node info is established, update the info. */
+        if (!wq_disable_numa) {
 
 
 
+            for_each_node_state(node, N_POSSIBLE)
+                cpumask_clear_cpu(cpu,
+                    wq_numa_possible_cpumask[node]);

Quoted 5 lines

Quoted 3 lines

The wq code try to reuse the origin pwqs/pools when the node still have cpu online. these 3 lines of code will cause the origin pwqs/pools be on the road of dying, and create a new set of pwqs/pools.

because the result of wq_calc_node_cpumask() changes ?

Yes.

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

Do you mean some comment should be added here ? or explaination for your reply for [3/4] ?

this fix [4/4] breaks the original design.

I'm sorry that I can't understand what this patch breaks.

the pwqs/pools should be kept if the node still have cpu online.

So, the fix's grand design should be

1. drop old pwq/pools only at node offline.
2. set proper pool->node based on online node info.
3. update pool->node of per-cpu-pool at cpu ONLINE.

Quoted 5 lines

Hm. (1) is done because cpumask_of_node() turns to be zero-filled after all cpus on a node offlined.

But, cpu-to-node relationship cannot be available until a cpu get onlined. It changes at every cpu onlining. So, at node online, refleshing cpumasks

It changes at every cpu being added which earlier than any cpu of the node will be online.

Quoted 1 line

of workqueues only after _all_ cpus on node are onlined seems to be the only way

When _any_ cpu on the new node is onlining, _add_ cpus of the node were *added*, which means all cpu_to_node()s of the all cpus on new node ware updated.

Unbound wq pool's node attribute is calculated at its allocation. But it's now calculated based on possible cpu<->node information which can be wrong after cpu hotplug/unplug.

If wrong pool->node is set, following allocation error will happen. ==
SLUB: Unable to allocate memory on node 2 (gfp=0x80d0)
cache: kmalloc-192, object size: 192, buffer size: 192, default order:
1, min order: 0
node 0: slabs: 6172, objs: 259224, free: 245741
node 1: slabs: 3261, objs: 136962, free: 127656
==

This patch fixes the node detection by making use of online cpu info. Unlike cpumask, the best node can be calculated by degree of overlap between attr->cpumask and numanode->online_cpumask.
This change doesn't corrupt original purpose of the old calculation.

Note: it's expected that this function is called as
pool_detect_best_node
get_unbound_pool
alloc_unbound_pwq
wq_update_unbound_numa
called at CPU_ONLINE/CPU_DOWN_PREPARE
and the latest online cpu info can be applied to a new wq pool, which replaces old one.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
 kernel/workqueue.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

kernel/workqueue.c +13 -0

index 09b685d..7809154 100644
@@ -3440,6 +3440,31 @@ static void put_unbound_pool(struct worker_pool *pool)
 }
 
 /**
+ * pool_detect_best_node - detect a node which contains specified cpumask.
+ * Should be called with wq_pool_mutex held.
+ * Returns a online node where the most of given cpus are tied to. 
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+	int node, best, match, selected = NUMA_NO_NODE;
+	static struct cpumask andmask; /* under wq_pool_mutex */
+
+	if (!wq_numa_enabled ||
+		cpumask_subset(cpu_online_mask, cpumask))
+		goto out;
+	best = 0;

kernel/workqueue.c +3 -12

+
+/**
  * get_unbound_pool - get a worker_pool with the specified attributes
  * @attrs: the attributes of the worker_pool to get
  *

@@ -3457,7 +3482,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
 {
 	u32 hash = wqattrs_hash(attrs);
 	struct worker_pool *pool;
-	int node;
 
 	lockdep_assert_held(&wq_pool_mutex);
 

@@ -3482,17 +3506,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
 	 * 'struct workqueue_attrs' comments for detail.
 	 */
 	pool->attrs->no_numa = false;
-
-	/* if cpumask is contained inside a NUMA node, we belong to that node */
-	if (wq_numa_enabled) {
-		for_each_node(node) {
-			if (cpumask_subset(pool->attrs->cpumask,
-					   wq_numa_possible_cpumask[node])) {
-				pool->node = node;
-				break;
-			}
-		}
-	}
+	pool->node = pool_detect_best_node(pool->attrs->cpumask);
 
 	if (worker_pool_assign_id(pool) < 0)
 		goto fail;

Quoted 58 lines · Kamezawa Hiroyuki

Unbound wq pool's node attribute is calculated at its allocation. But it's now calculated based on possible cpu<->node information which can be wrong after cpu hotplug/unplug.

If wrong pool->node is set, following allocation error will happen. ==
SLUB: Unable to allocate memory on node 2 (gfp=0x80d0)
cache: kmalloc-192, object size: 192, buffer size: 192, default order:
1, min order: 0
node 0: slabs: 6172, objs: 259224, free: 245741
node 1: slabs: 3261, objs: 136962, free: 127656
==

This patch fixes the node detection by making use of online cpu info. Unlike cpumask, the best node can be calculated by degree of overlap between attr->cpumask and numanode->online_cpumask.
This change doesn't corrupt original purpose of the old calculation.

Note: it's expected that this function is called as
pool_detect_best_node
get_unbound_pool
alloc_unbound_pwq
wq_update_unbound_numa
called at CPU_ONLINE/CPU_DOWN_PREPARE
and the latest online cpu info can be applied to a new wq pool, which replaces old one.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
  kernel/workqueue.c | 38 ++++++++++++++++++++++++++------------
  1 file changed, 26 insertions(+), 12 deletions(-)

kernel/workqueue.c +18 -0

index 09b685d..7809154 100644
@@ -3440,6 +3440,31 @@ static void put_unbound_pool(struct worker_pool *pool)
  }
 
  /**
+ * pool_detect_best_node - detect a node which contains specified cpumask.
+ * Should be called with wq_pool_mutex held.
+ * Returns a online node where the most of given cpus are tied to.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+	int node, best, match, selected = NUMA_NO_NODE;
+	static struct cpumask andmask; /* under wq_pool_mutex */
+
+	if (!wq_numa_enabled ||
+		cpumask_subset(cpu_online_mask, cpumask))
+		goto out;
+	best = 0;
+	/* select a node which contains the most number of cpu */
+	for_each_node_state(node, N_ONLINE) {
+		cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+		match = cpumask_weight(&andmask);
+		if (match > best)

kernel/workqueue.c static void put_unbound_pool(struct worker_pool *pool) · line 3465

Quoted 5 lines

kernel/workqueue.c +5 -0

+			selected = best;
+	}
+out:
+	return selected;
+}

This is a mixture of fix and development. Why not just keep the original calculation?

if the mask cover multiple nodes, NUMA_NO_NODE is the best for pool->node after the pool was created. The memory allocation will select the best node for manage_workers(), from which CPU that the worker actually is running on.

Quoted 59 lines · Lai Jiangshan

Quoted 58 lines · Kamezawa Hiroyuki

Unbound wq pool's node attribute is calculated at its allocation. But it's now calculated based on possible cpu<->node information which can be wrong after cpu hotplug/unplug.

If wrong pool->node is set, following allocation error will happen. ==
SLUB: Unable to allocate memory on node 2 (gfp=0x80d0)
cache: kmalloc-192, object size: 192, buffer size: 192, default order:
1, min order: 0
node 0: slabs: 6172, objs: 259224, free: 245741
node 1: slabs: 3261, objs: 136962, free: 127656
==

This patch fixes the node detection by making use of online cpu info. Unlike cpumask, the best node can be calculated by degree of overlap between attr->cpumask and numanode->online_cpumask.
This change doesn't corrupt original purpose of the old calculation.

Note: it's expected that this function is called as
pool_detect_best_node
get_unbound_pool
alloc_unbound_pwq
wq_update_unbound_numa
called at CPU_ONLINE/CPU_DOWN_PREPARE
and the latest online cpu info can be applied to a new wq pool, which replaces old one.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
  kernel/workqueue.c | 38 ++++++++++++++++++++++++++------------
  1 file changed, 26 insertions(+), 12 deletions(-)

kernel/workqueue.c +18 -0

index 09b685d..7809154 100644
@@ -3440,6 +3440,31 @@ static void put_unbound_pool(struct worker_pool *pool)
  }
 
  /**
+ * pool_detect_best_node - detect a node which contains specified cpumask.
+ * Should be called with wq_pool_mutex held.
+ * Returns a online node where the most of given cpus are tied to.
+ */
+static int pool_detect_best_node(const struct cpumask *cpumask)
+{
+    int node, best, match, selected = NUMA_NO_NODE;
+    static struct cpumask andmask; /* under wq_pool_mutex */
+
+    if (!wq_numa_enabled ||
+        cpumask_subset(cpu_online_mask, cpumask))
+        goto out;
+    best = 0;
+    /* select a node which contains the most number of cpu */
+    for_each_node_state(node, N_ONLINE) {
+        cpumask_and(&andmask, cpumask, cpumask_of_node(node));
+        match = cpumask_weight(&andmask);
+        if (match > best)

Quoted 9 lines

kernel/workqueue.c static void put_unbound_pool(struct worker_pool *pool) · line 3465

Quoted 5 lines

kernel/workqueue.c +5 -0

+            selected = best;
+    }
+out:
+    return selected;
+}

This is a mixture of fix and development. Why not just keep the original calculation?

Just because wq_numa_possible_mask is broken,

it shouldn't be used if possible.
In this patch series, the mask is updated only when a node is coming up. Is it better to clear it at node offline ?

Quoted 4 lines

if the mask cover multiple nodes, NUMA_NO_NODE is the best for pool->node after the pool was created. The memory allocation will select the best node for manage_workers(), from which CPU that the worker actually is running on.

I'll drop this and try to keep original code as much as possible.

Thanks,
-Kame

.

The percpu workqueue pool are persistend and never be freed. But cpu<->node relationship can be changed by cpu hotplug and pool->node can point to an offlined node.

If pool->node points to an offlined node,
following allocation failure can happen.

    ==
     SLUB: Unable to allocate memory on node 2 (gfp=0x80d0)
      cache: kmalloc-192, object size: 192, buffer size: 192, default
order:
    1, min order: 0
      node 0: slabs: 6172, objs: 259224, free: 245741
      node 1: slabs: 3261, objs: 136962, free: 127656
    ==

This patch clears per-cpu workqueue pool's node affinity at cpu offlining and restore it at cpu onlining.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
 kernel/workqueue.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

kernel/workqueue.c +6 -0

index 7809154..2fd0bd7 100644
@@ -4586,6 +4586,11 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
 	case CPU_DOWN_FAILED:
 	case CPU_ONLINE:
 		mutex_lock(&wq_pool_mutex);
+		/*
+		 * now cpu <-> node info is established. update numa node
+		 */
+		for_each_cpu_worker_pool(pool, cpu)
+			pool->node = cpu_to_node(cpu);
 
 		for_each_pool(pool, pi) {
 			mutex_lock(&pool->attach_mutex);

@@ -4619,6 +4624,7 @@ static int workqueue_cpu_down_callback(struct notifier_block *nfb,
 	int cpu = (unsigned long)hcpu;
 	struct work_struct unbind_work;
 	struct workqueue_struct *wq;
+	struct worker_pool *pool;
 
 	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_DOWN_PREPARE:

@@ -4626,10 +4632,13 @@ static int workqueue_cpu_down_callback(struct notifier_block *nfb,
 		INIT_WORK_ONSTACK(&unbind_work, wq_unbind_fn);

kernel/workqueue.c +0 -0

 		mutex_unlock(&wq_pool_mutex);
 
 		/* wait for per-cpu unbinding to finish */

Quoted 51 lines · Kamezawa Hiroyuki

The percpu workqueue pool are persistend and never be freed. But cpu<->node relationship can be changed by cpu hotplug and pool->node can point to an offlined node.

If pool->node points to an offlined node,
following allocation failure can happen.

     ==
      SLUB: Unable to allocate memory on node 2 (gfp=0x80d0)
       cache: kmalloc-192, object size: 192, buffer size: 192, default
order:
     1, min order: 0
       node 0: slabs: 6172, objs: 259224, free: 245741
       node 1: slabs: 3261, objs: 136962, free: 127656
     ==

This patch clears per-cpu workqueue pool's node affinity at cpu offlining and restore it at cpu onlining.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
  kernel/workqueue.c | 11 ++++++++++-
  1 file changed, 10 insertions(+), 1 deletion(-)

kernel/workqueue.c +6 -0

index 7809154..2fd0bd7 100644
@@ -4586,6 +4586,11 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
  	case CPU_DOWN_FAILED:
  	case CPU_ONLINE:
  		mutex_lock(&wq_pool_mutex);
+		/*
+		 * now cpu <-> node info is established. update numa node
+		 */
+		for_each_cpu_worker_pool(pool, cpu)
+			pool->node = cpu_to_node(cpu);
 
  		for_each_pool(pool, pi) {
  			mutex_lock(&pool->attach_mutex);

@@ -4619,6 +4624,7 @@ static int workqueue_cpu_down_callback(struct notifier_block *nfb,
  	int cpu = (unsigned long)hcpu;
  	struct work_struct unbind_work;
  	struct workqueue_struct *wq;
+	struct worker_pool *pool;
 
  	switch (action & ~CPU_TASKS_FROZEN) {
  	case CPU_DOWN_PREPARE:

@@ -4626,10 +4632,13 @@ static int workqueue_cpu_down_callback(struct notifier_block *nfb,
  		INIT_WORK_ONSTACK(&unbind_work, wq_unbind_fn);
  		queue_work_on(cpu, system_highpri_wq, &unbind_work);
 

kernel/workqueue.c static int workqueue_cpu_down_callback(struct notifier_block *nfb, · line 4641

Quoted 8 lines

kernel/workqueue.c +4 -1

-		/* update NUMA affinity of unbound workqueues */
  		mutex_lock(&wq_pool_mutex);
+		/* update NUMA affinity of unbound workqueues */
  		list_for_each_entry(wq, &workqueues, list)
  			wq_update_unbound_numa(wq, cpu, false);
+		/* clear per-cpu workqueues's numa affinity. */
+		for_each_cpu_worker_pool(pool, cpu)
+			pool->node = NUMA_NO_NODE; /* restored at online */

If the node is still online, it is better to keep the original pool->node.

Workqueue keeps cpu<->node relationship including all possible cpus. The original information was made at boot but it may change when a new node is added.

Update information if a new node is ready with using node-hotplug callback.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
 include/linux/memory_hotplug.h |  2 ++
 kernel/workqueue.c             | 28 +++++++++++++++++++++++++++-
 mm/memory_hotplug.c            |  4 ++++
 3 files changed, 33 insertions(+), 1 deletion(-)

include/linux/memory_hotplug.h +2 -0

index 8f1a419..cd3cb67 100644
@@ -270,4 +270,6 @@ extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms)
 extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
 					  unsigned long pnum);
 
+/* update for workqueues */
+void workqueue_node_register(int node);
 #endif /* __LINUX_MEMORY_HOTPLUG_H */

kernel/workqueue.c +1 -1

index 2fd0bd7..5499b76 100644
@@ -266,7 +266,7 @@ struct workqueue_struct {
 static struct kmem_cache *pwq_cache;
 
 static cpumask_var_t *wq_numa_possible_cpumask;
-					/* possible CPUs of each node */
+					/* PL: possible CPUs of each node */
 
 static bool wq_disable_numa;
 module_param_named(disable_numa, wq_disable_numa, bool, 0444);

kernel/workqueue.c +19 -0

+	cpumask_set_cpu(cpu, wq_numa_possible_cpumask[node]);
+}
+/*
+ * When a cpu is physically added, cpu<->node relationship is established.
+ * We can catch the whole view when a new NODE_DATA() coming up.
+ * Update cpumask used for sched affinity of workers.
+ */
+void workqueue_node_register(int node)
+{
+	int cpu;
+	mutex_lock(&wq_pool_mutex);
+	for_each_possible_cpu(cpu) {
+		if (node == cpu_to_node(cpu))
+			reflesh_wq_possible_mask(cpu, node);
+	}
+	mutex_unlock(&wq_pool_mutex);
+}
+
+#endif
 
 /*
  * Workqueues should be brought up before normal priority CPU notifiers.

mm/memory_hotplug.c +4 -0

index 1bf4807..d0c1ebb 100644
@@ -1121,6 +1121,10 @@ static pg_data_t __ref *hotadd_new_pgdat(int nid, u64 start)
 	 * online_pages() and offline_pages().
 	 */
 	reset_node_present_pages(pgdat);
+	/*
+	 * Update workqueue's cpu/node affinity info.
+	 */
+	workqueue_node_register(nid);
 
 	return pgdat;
 }

Quoted 35 lines · Kamezawa Hiroyuki

Workqueue keeps cpu<->node relationship including all possible cpus. The original information was made at boot but it may change when a new node is added.

Update information if a new node is ready with using node-hotplug callback.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
  include/linux/memory_hotplug.h |  2 ++
  kernel/workqueue.c             | 28 +++++++++++++++++++++++++++-
  mm/memory_hotplug.c            |  4 ++++
  3 files changed, 33 insertions(+), 1 deletion(-)

include/linux/memory_hotplug.h +2 -0

index 8f1a419..cd3cb67 100644
@@ -270,4 +270,6 @@ extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms)
  extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
  					  unsigned long pnum);
 
+/* update for workqueues */
+void workqueue_node_register(int node);
  #endif /* __LINUX_MEMORY_HOTPLUG_H */

kernel/workqueue.c +1 -1

index 2fd0bd7..5499b76 100644
@@ -266,7 +266,7 @@ struct workqueue_struct {
  static struct kmem_cache *pwq_cache;
 
  static cpumask_var_t *wq_numa_possible_cpumask;
-					/* possible CPUs of each node */
+					/* PL: possible CPUs of each node */
 

kernel/workqueue.c static void restore_unbound_workers_cpumask(struct worker_pool *pool, int cpu) · line 4568

Quoted 13 lines

kernel/workqueue.c +7 -0

  static bool wq_disable_numa;
  module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -4559,6 +4559,32 @@ static void restore_unbound_workers_cpumask(struct worker_pool *pool, int cpu)
  		WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task,
  						  pool->attrs->cpumask) < 0);
  }
+#ifdef CONFIG_MEMORY_HOTPLUG
+
+static void reflesh_wq_possible_mask(int cpu, int node)
+{
+	int oldnode;
+	for_each_node_state(oldnode, N_POSSIBLE)
+		cpumask_clear_cpu(cpu, wq_numa_possible_cpumask[oldnode]);

You need to check and update all the wq->numa_pwq_tbl[oldnode]

Quoted 37 lines

kernel/workqueue.c +19 -0

+	cpumask_set_cpu(cpu, wq_numa_possible_cpumask[node]);
+}
+/*
+ * When a cpu is physically added, cpu<->node relationship is established.
+ * We can catch the whole view when a new NODE_DATA() coming up.
+ * Update cpumask used for sched affinity of workers.
+ */
+void workqueue_node_register(int node)
+{
+	int cpu;
+	mutex_lock(&wq_pool_mutex);
+	for_each_possible_cpu(cpu) {
+		if (node == cpu_to_node(cpu))
+			reflesh_wq_possible_mask(cpu, node);
+	}
+	mutex_unlock(&wq_pool_mutex);
+}
+
+#endif
 
  /*
   * Workqueues should be brought up before normal priority CPU notifiers.

mm/memory_hotplug.c +4 -0

index 1bf4807..d0c1ebb 100644
@@ -1121,6 +1121,10 @@ static pg_data_t __ref *hotadd_new_pgdat(int nid, u64 start)
  	 * online_pages() and offline_pages().
  	 */
  	reset_node_present_pages(pgdat);
+	/*
+	 * Update workqueue's cpu/node affinity info.
+	 */
+	workqueue_node_register(nid);
 
  	return pgdat;
  }

Quoted 36 lines · Lai Jiangshan

Quoted 35 lines · Kamezawa Hiroyuki

Workqueue keeps cpu<->node relationship including all possible cpus. The original information was made at boot but it may change when a new node is added.

Update information if a new node is ready with using node-hotplug callback.

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
  include/linux/memory_hotplug.h |  2 ++
  kernel/workqueue.c             | 28 +++++++++++++++++++++++++++-
  mm/memory_hotplug.c            |  4 ++++
  3 files changed, 33 insertions(+), 1 deletion(-)

include/linux/memory_hotplug.h +2 -0

index 8f1a419..cd3cb67 100644
@@ -270,4 +270,6 @@ extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms)
  extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
                        unsigned long pnum);
 
+/* update for workqueues */
+void workqueue_node_register(int node);
  #endif /* __LINUX_MEMORY_HOTPLUG_H */

kernel/workqueue.c +1 -1

index 2fd0bd7..5499b76 100644
@@ -266,7 +266,7 @@ struct workqueue_struct {
  static struct kmem_cache *pwq_cache;
 
  static cpumask_var_t *wq_numa_possible_cpumask;
-                    /* possible CPUs of each node */
+                    /* PL: possible CPUs of each node */
 

Quoted 16 lines

kernel/workqueue.c static void restore_unbound_workers_cpumask(struct worker_pool *pool, int cpu) · line 4568

Quoted 13 lines

kernel/workqueue.c +7 -0

  static bool wq_disable_numa;
  module_param_named(disable_numa, wq_disable_numa, bool, 0444);

@@ -4559,6 +4559,32 @@ static void restore_unbound_workers_cpumask(struct worker_pool *pool, int cpu)
          WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task,
                            pool->attrs->cpumask) < 0);
  }
+#ifdef CONFIG_MEMORY_HOTPLUG
+
+static void reflesh_wq_possible_mask(int cpu, int node)
+{
+    int oldnode;
+    for_each_node_state(oldnode, N_POSSIBLE)
+        cpumask_clear_cpu(cpu, wq_numa_possible_cpumask[oldnode]);

You need to check and update all the wq->numa_pwq_tbl[oldnode]

you mean : if I drop patch 1/4, it will be required. Right ? I'll add that check.