On 12/16/2014 03:32 PM, Kamezawa Hiroyuki wrote: > (2014/12/16 14:30), Lai Jiangshan wrote: >> On 12/15/2014 07:14 PM, Kamezawa Hiroyuki wrote: >>> 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 <kamezawa.hir...@jp.fujitsu.com> >>> --- >>> kernel/workqueue.c | 38 ++++++++++++++++++++++++++------------ >>> 1 file changed, 26 insertions(+), 12 deletions(-) >>> >>> diff --git a/kernel/workqueue.c b/kernel/workqueue.c >>> index 09b685d..7809154 100644 >>> --- a/kernel/workqueue.c >>> +++ b/kernel/workqueue.c >>> @@ -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) >>> + 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 is not broken if the bug is fixed. > 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 ? > >> 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 > > > . > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/