[PATCH] kthread: ensure locality of task_struct allocations

Nishanth Aravamudan

author

2014-01-28 18:38 UTC #0 lore raw

PATCH 1/1 kthread: ensure locality of task_struct allocations

In the presence of memoryless nodes, numa_node_id()/cpu_to_node() will return the current CPU's NUMA node, but that may not be where we expect to allocate from memory from. Instead, we should use numa_mem_id()/cpu_to_mem(). On one ppc64 system with a memoryless Node 0, this ends up saving nearly 500M of slab due to less fragmentation.

Signed-off-by Nishanth Aravamudan <[email protected]>

kernel/kthread.c +2 -2

index b5ae3ee..8573e4e 100644
@@ -217,7 +217,7 @@ int tsk_fork_get_node(struct task_struct *tsk)
 	if (tsk == kthreadd_task)
 		return tsk->pref_node_fork;
 #endif
-	return numa_node_id();
+	return numa_mem_id();
 }
 
 static void create_kthread(struct kthread_create_info *create)

@@ -369,7 +369,7 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
 {
 	struct task_struct *p;
 
-	p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu), namefmt,
+	p = kthread_create_on_node(threadfn, data, cpu_to_mem(cpu), namefmt,
 				   cpu);
 	if (IS_ERR(p))
 		return p;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

Acked-by on v1 1/1

2014-01-29 08:13 UTC #1 lore raw

Quoted 7 lines · Nishanth Aravamudan

In the presence of memoryless nodes, numa_node_id()/cpu_to_node() will return the current CPU's NUMA node, but that may not be where we expect to allocate from memory from. Instead, we should use numa_mem_id()/cpu_to_mem(). On one ppc64 system with a memoryless Node 0, this ends up saving nearly 500M of slab due to less fragmentation.
Signed-off-by Nishanth Aravamudan <[email protected]>

Acked-by David Rientjes <[email protected]>

kernel/kthread.c int tsk_fork_get_node(struct task_struct *tsk) · line 220

Quoted 10 lines

kernel/kthread.c +1 -1

index b5ae3ee..8573e4e 100644
@@ -217,7 +217,7 @@ int tsk_fork_get_node(struct task_struct *tsk)
 	if (tsk == kthreadd_task)
 		return tsk->pref_node_fork;
 #endif
-	return numa_node_id();
+	return numa_mem_id();

I'm wondering why return NUMA_NO_NODE wouldn't have the same effect and prefer the local node?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2014-01-29 15:58 UTC #2 lore raw

Quoted 5 lines · David Rientjes

Quoted 5 lines

kernel/kthread.c +0 -0

index b5ae3ee..8573e4e 100644
@@ -217,7 +217,7 @@ int tsk_fork_get_node(struct task_struct *tsk)

Quoted 9 lines

kernel/kthread.c int tsk_fork_get_node(struct task_struct *tsk) · line 220
Quoted 5 lines
kernel/kthread.c +1 -1
 	if (tsk == kthreadd_task)
 		return tsk->pref_node_fork;
 #endif
-	return numa_node_id();
+	return numa_mem_id();
I'm wondering why return NUMA_NO_NODE wouldn't have the same effect and prefer the local node?

The idea here seems to be that the allocation may occur from a cpu that is different from where the process will run later on.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

2014-01-30 00:27 UTC #3 lore raw

Quoted 15 lines · Christoph Lameter

Quoted 5 lines
Quoted 5 lines
kernel/kthread.c +0 -0
index b5ae3ee..8573e4e 100644
@@ -217,7 +217,7 @@ int tsk_fork_get_node(struct task_struct *tsk)
Quoted 9 lines
kernel/kthread.c int tsk_fork_get_node(struct task_struct *tsk) · line 220
Quoted 5 lines
kernel/kthread.c +1 -1
 	if (tsk == kthreadd_task)
 		return tsk->pref_node_fork;
 #endif
-	return numa_node_id();
+	return numa_mem_id();
I'm wondering why return NUMA_NO_NODE wouldn't have the same effect and prefer the local node?

Quoted 3 lines

The idea here seems to be that the allocation may occur from a cpu that is different from where the process will run later on.

Yeah, that makes sense for kthreadd, but I'm wondering why we have to return numa_mem_id() rather than just NUMA_NO_NODE. Sorry for not being specific about doing s/numa_mem_id/NUMA_NO_NODE/ here.

That should just turn kmem_cache_alloc_node() into kmem_cache_alloc() and alloc_pages_node() into alloc_pages() for the allocators that use this return value, task_struct and thread_info. If that's not allocating local memory, if possible, and numa_mem_id() magically does, then there's a problem.

Eric, did you try this when writing 207205a2ba26 ("kthread: NUMA aware kthread_create_on_node()") or was it always numa_node_id() from the beginning?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Eric Dumazet

2014-01-30 06:14 UTC #4 lore raw

Quoted 3 lines · David Rientjes

Eric, did you try this when writing 207205a2ba26 ("kthread: NUMA aware kthread_create_on_node()") or was it always numa_node_id() from the beginning?

Hmm, I think I did not try this, its absolutely possible NUMA_NO_NODE was better here.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

2014-01-30 22:47 UTC #5 lore raw

Quoted 7 lines · Eric Dumazet

Quoted 3 lines
Eric, did you try this when writing 207205a2ba26 ("kthread: NUMA aware kthread_create_on_node()") or was it always numa_node_id() from the beginning?
Hmm, I think I did not try this, its absolutely possible NUMA_NO_NODE was better here.

Nishanth, could you change your patch to just return NUMA_NO_NODE for the non-kthreadd case?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Nishanth Aravamudan

author

2014-01-30 23:08 UTC #6 lore raw

PATCH 1/1 kthread: ensure locality of task_struct allocations

Quoted 6 lines · David Rientjes

Quoted 4 lines · Eric Dumazet
Quoted 3 lines
Eric, did you try this when writing 207205a2ba26 ("kthread: NUMA aware kthread_create_on_node()") or was it always numa_node_id() from the beginning?

Quoted 6 lines

Quoted 3 lines
Hmm, I think I did not try this, its absolutely possible NUMA_NO_NODE was better here.
Nishanth, could you change your patch to just return NUMA_NO_NODE for the non-kthreadd case?

Something like the following?

In the presence of memoryless nodes, numa_node_id() will return the current CPU's NUMA node, but that may not be where we expect to allocate from memory from. Instead, we should rely on the fallback code in the memory allocator itself, by using NUMA_NO_NODE. Also, when calling kthread_create_on_node(), use the nearest node with memory to the cpu in question, rather than the node it is running on.

Signed-off-by Nishanth Aravamudan <[email protected]>

Cc Anton Blanchard <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Oleg Nesterov <[email protected]>

Cc Jan Kara <[email protected]>

Cc David Rientjes <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Tetsuo Handa <[email protected]>

Cc [email protected]

Cc Wanpeng Li <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Ben Herrenschmidt <[email protected]>

---
Note that I haven't yet tested this change on the system that reproduce the original problem yet.

kernel/kthread.c +2 -2

index b5ae3ee..9a130ec 100644
@@ -217,7 +217,7 @@ int tsk_fork_get_node(struct task_struct *tsk)
 	if (tsk == kthreadd_task)
 		return tsk->pref_node_fork;
 #endif
-	return numa_node_id();
+	return NUMA_NO_NODE;
 }
 
 static void create_kthread(struct kthread_create_info *create)

@@ -369,7 +369,7 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
 {
 	struct task_struct *p;
 
-	p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu), namefmt,
+	p = kthread_create_on_node(threadfn, data, cpu_to_mem(cpu), namefmt,
 				   cpu);
 	if (IS_ERR(p))
 		return p;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

Acked-by on v1 1/1

2014-01-30 23:31 UTC #7 lore raw

Quoted 8 lines · Nishanth Aravamudan

In the presence of memoryless nodes, numa_node_id() will return the current CPU's NUMA node, but that may not be where we expect to allocate from memory from. Instead, we should rely on the fallback code in the memory allocator itself, by using NUMA_NO_NODE. Also, when calling kthread_create_on_node(), use the nearest node with memory to the cpu in question, rather than the node it is running on.
Signed-off-by Nishanth Aravamudan <[email protected]>

Acked-by David Rientjes <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

Reviewed-by on v1 1/1

2014-01-31 15:14 UTC #8 lore raw

In reply to Nishanth Aravamudan

Quoted 6 lines · Nishanth Aravamudan

In the presence of memoryless nodes, numa_node_id() will return the current CPU's NUMA node, but that may not be where we expect to allocate from memory from. Instead, we should rely on the fallback code in the memory allocator itself, by using NUMA_NO_NODE. Also, when calling kthread_create_on_node(), use the nearest node with memory to the cpu in question, rather than the node it is running on.

Looks good to me.

Reviewed-by Christoph Lameter <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

Reviewed-by on v1 1/1

2014-01-29 15:57 UTC #9 lore raw

In reply to Nishanth Aravamudan

Quoted 5 lines · Nishanth Aravamudan

In the presence of memoryless nodes, numa_node_id()/cpu_to_node() will return the current CPU's NUMA node, but that may not be where we expect to allocate from memory from. Instead, we should use numa_mem_id()/cpu_to_mem(). On one ppc64 system with a memoryless Node 0, this ends up saving nearly 500M of slab due to less fragmentation.

Reviewed-by Christoph Lameter <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>