[PATCH] mm/khugepaged: sched to numa node when collapse huge page

[PATCH] mm/khugepaged: sched to numa node when collapse huge page

[Bibo Mao]

collapse huge page is slow, specially when khugepaged daemon runs
on different numa node with that of huge page. It suffers from
huge page copying across nodes, also cache is not used for target
node. With this patch, khugepaged daemon switches to the same numa
node with huge page. It saves copying time and makes use of local
cache better.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 mm/khugepaged.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 131492fd1148..460c285dc974 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -116,6 +116,7 @@ struct khugepaged_scan {
 	struct list_head mm_head;
 	struct mm_slot *mm_slot;
 	unsigned long address;
+	int node;
 };
 
 static struct khugepaged_scan khugepaged_scan = {
@@ -1066,6 +1067,7 @@ static void collapse_huge_page(struct mm_struct *mm,
 	struct vm_area_struct *vma;
 	struct mmu_notifier_range range;
 	gfp_t gfp;
+	const struct cpumask *cpumask;
 
 	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
 
@@ -1079,6 +1081,13 @@ static void collapse_huge_page(struct mm_struct *mm,
 	 * that. We will recheck the vma after taking it again in write mode.
 	 */
 	mmap_read_unlock(mm);
+
+	/* sched to specified node before huage page memory copy */
+	cpumask = cpumask_of_node(node);
+	if ((khugepaged_scan.node != node) && !cpumask_empty(cpumask)) {
+		set_cpus_allowed_ptr(current, cpumask);
+		khugepaged_scan.node = node;
+	}
 	new_page = khugepaged_alloc_page(hpage, gfp, node);
 	if (!new_page) {
 		result = SCAN_ALLOC_HUGE_PAGE_FAIL;
@@ -2380,6 +2389,7 @@ int start_stop_khugepaged(void)
 		kthread_stop(khugepaged_thread);
 		khugepaged_thread = NULL;
 	}
+	khugepaged_scan.node = NUMA_NO_NODE;
 	set_recommended_min_free_kbytes();
 fail:
 	mutex_unlock(&khugepaged_mutex);
-- 
2.31.1

Re: [PATCH] mm/khugepaged: sched to numa node when collapse huge page

[David Hildenbrand]

On 11.03.22 10:01, Bibo Mao wrote:
> collapse huge page is slow, specially when khugepaged daemon runs
> on different numa node with that of huge page. It suffers from
> huge page copying across nodes, also cache is not used for target
> node. With this patch, khugepaged daemon switches to the same numa
> node with huge page. It saves copying time and makes use of local
> cache better.

Hi,

just the usual question, do you have any performance numbers to back
your claims (e.g., "is slow, specially when") and proof that this patch
does the trick?


> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  mm/khugepaged.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 131492fd1148..460c285dc974 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -116,6 +116,7 @@ struct khugepaged_scan {
>  	struct list_head mm_head;
>  	struct mm_slot *mm_slot;
>  	unsigned long address;
> +	int node;
>  };
>  
>  static struct khugepaged_scan khugepaged_scan = {
> @@ -1066,6 +1067,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>  	struct vm_area_struct *vma;
>  	struct mmu_notifier_range range;
>  	gfp_t gfp;
> +	const struct cpumask *cpumask;

We tend to stick to reverse Christmas tree format as good as possible.

>  
>  	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
>  
> @@ -1079,6 +1081,13 @@ static void collapse_huge_page(struct mm_struct *mm,
>  	 * that. We will recheck the vma after taking it again in write mode.
>  	 */
>  	mmap_read_unlock(mm);
> +
> +	/* sched to specified node before huage page memory copy */

s/huage/huge/

> +	cpumask = cpumask_of_node(node);
> +	if ((khugepaged_scan.node != node) && !cpumask_empty(cpumask)) {
> +		set_cpus_allowed_ptr(current, cpumask);
> +		khugepaged_scan.node = node;
> +	}
>  	new_page = khugepaged_alloc_page(hpage, gfp, node);
>  	if (!new_page) {
>  		result = SCAN_ALLOC_HUGE_PAGE_FAIL;
> @@ -2380,6 +2389,7 @@ int start_stop_khugepaged(void)
>  		kthread_stop(khugepaged_thread);
>  		khugepaged_thread = NULL;
>  	}
> +	khugepaged_scan.node = NUMA_NO_NODE;
>  	set_recommended_min_free_kbytes();
>  fail:
>  	mutex_unlock(&khugepaged_mutex);


-- 
Thanks,

David / dhildenb

Re: [PATCH] mm/khugepaged: sched to numa node when collapse huge page

[maobibo]

On 03/11/2022 05:20 PM, David Hildenbrand wrote:
> On 11.03.22 10:01, Bibo Mao wrote:
>> collapse huge page is slow, specially when khugepaged daemon runs
>> on different numa node with that of huge page. It suffers from
>> huge page copying across nodes, also cache is not used for target
>> node. With this patch, khugepaged daemon switches to the same numa
>> node with huge page. It saves copying time and makes use of local
>> cache better.
> 
> Hi,
> 
> just the usual question, do you have any performance numbers to back
> your claims (e.g., "is slow, specially when") and proof that this patch
> does the trick?
With specint 2006 on loongarch 3C5000L 32core numa system, it improves
about 6%. The page size is 16K and pmd page size is 32M, memory performance
across numa node is obvious different. However I do not test it on x86 box.


> 
> 
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>  mm/khugepaged.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 131492fd1148..460c285dc974 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -116,6 +116,7 @@ struct khugepaged_scan {
>>  	struct list_head mm_head;
>>  	struct mm_slot *mm_slot;
>>  	unsigned long address;
>> +	int node;
>>  };
>>  
>>  static struct khugepaged_scan khugepaged_scan = {
>> @@ -1066,6 +1067,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>>  	struct vm_area_struct *vma;
>>  	struct mmu_notifier_range range;
>>  	gfp_t gfp;
>> +	const struct cpumask *cpumask;
> 
> We tend to stick to reverse Christmas tree format as good as possible.
> 
>>  
>>  	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
>>  
>> @@ -1079,6 +1081,13 @@ static void collapse_huge_page(struct mm_struct *mm,
>>  	 * that. We will recheck the vma after taking it again in write mode.
>>  	 */
>>  	mmap_read_unlock(mm);
>> +
>> +	/* sched to specified node before huage page memory copy */
> 
> s/huage/huge/
> 
>> +	cpumask = cpumask_of_node(node);
>> +	if ((khugepaged_scan.node != node) && !cpumask_empty(cpumask)) {
>> +		set_cpus_allowed_ptr(current, cpumask);
>> +		khugepaged_scan.node = node;
>> +	}
>>  	new_page = khugepaged_alloc_page(hpage, gfp, node);
>>  	if (!new_page) {
>>  		result = SCAN_ALLOC_HUGE_PAGE_FAIL;
>> @@ -2380,6 +2389,7 @@ int start_stop_khugepaged(void)
>>  		kthread_stop(khugepaged_thread);
>>  		khugepaged_thread = NULL;
>>  	}
>> +	khugepaged_scan.node = NUMA_NO_NODE;
>>  	set_recommended_min_free_kbytes();
>>  fail:
>>  	mutex_unlock(&khugepaged_mutex);
> 
> 

Re: [PATCH] mm/khugepaged: sched to numa node when collapse huge page

[David Hildenbrand]

On 11.03.22 10:51, maobibo wrote:
> 
> 
> On 03/11/2022 05:20 PM, David Hildenbrand wrote:
>> On 11.03.22 10:01, Bibo Mao wrote:
>>> collapse huge page is slow, specially when khugepaged daemon runs
>>> on different numa node with that of huge page. It suffers from
>>> huge page copying across nodes, also cache is not used for target
>>> node. With this patch, khugepaged daemon switches to the same numa
>>> node with huge page. It saves copying time and makes use of local
>>> cache better.
>>
>> Hi,
>>
>> just the usual question, do you have any performance numbers to back
>> your claims (e.g., "is slow, specially when") and proof that this patch
>> does the trick?
> With specint 2006 on loongarch 3C5000L 32core numa system, it improves
> about 6%. The page size is 16K and pmd page size is 32M, memory performance
> across numa node is obvious different. However I do not test it on x86 box.
> 

Thanks, can you add these details to the patch description?


-- 
Thanks,

David / dhildenb

Re: [PATCH] mm/khugepaged: sched to numa node when collapse huge page

[maobibo]

On 03/11/2022 05:55 PM, David Hildenbrand wrote:
> On 11.03.22 10:51, maobibo wrote:
>>
>>
>> On 03/11/2022 05:20 PM, David Hildenbrand wrote:
>>> On 11.03.22 10:01, Bibo Mao wrote:
>>>> collapse huge page is slow, specially when khugepaged daemon runs
>>>> on different numa node with that of huge page. It suffers from
>>>> huge page copying across nodes, also cache is not used for target
>>>> node. With this patch, khugepaged daemon switches to the same numa
>>>> node with huge page. It saves copying time and makes use of local
>>>> cache better.
>>>
>>> Hi,
>>>
>>> just the usual question, do you have any performance numbers to back
>>> your claims (e.g., "is slow, specially when") and proof that this patch
>>> does the trick?
>> With specint 2006 on loongarch 3C5000L 32core numa system, it improves
>> about 6%. The page size is 16K and pmd page size is 32M, memory performance
>> across numa node is obvious different. However I do not test it on x86 box.
>>
> 
> Thanks, can you add these details to the patch description?
Surely, will do in next patch.

regards
bibo, mao
> 
> 

Re: [PATCH] mm/khugepaged: sched to numa node when collapse huge page

[Yang Shi]

On Fri, Mar 11, 2022 at 1:01 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
> collapse huge page is slow, specially when khugepaged daemon runs
> on different numa node with that of huge page. It suffers from
> huge page copying across nodes, also cache is not used for target
> node. With this patch, khugepaged daemon switches to the same numa
> node with huge page. It saves copying time and makes use of local
> cache better.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  mm/khugepaged.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 131492fd1148..460c285dc974 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -116,6 +116,7 @@ struct khugepaged_scan {
>         struct list_head mm_head;
>         struct mm_slot *mm_slot;
>         unsigned long address;
> +       int node;
>  };
>
>  static struct khugepaged_scan khugepaged_scan = {
> @@ -1066,6 +1067,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>         struct vm_area_struct *vma;
>         struct mmu_notifier_range range;
>         gfp_t gfp;
> +       const struct cpumask *cpumask;
>
>         VM_BUG_ON(address & ~HPAGE_PMD_MASK);
>
> @@ -1079,6 +1081,13 @@ static void collapse_huge_page(struct mm_struct *mm,
>          * that. We will recheck the vma after taking it again in write mode.
>          */
>         mmap_read_unlock(mm);
> +
> +       /* sched to specified node before huage page memory copy */
> +       cpumask = cpumask_of_node(node);
> +       if ((khugepaged_scan.node != node) && !cpumask_empty(cpumask)) {
> +               set_cpus_allowed_ptr(current, cpumask);
> +               khugepaged_scan.node = node;

What if khugepaged was scheduled to the other nodes after this, but
khugepaged_scan.node still equals to node? It seems possible to me
IIUC.

TBH I'm not quite sure if migrating khugepaged is really worth it for
everyone or not. The worst case is the locality of base pages are not
obvious, for example, the base pages may be across all nodes, so you
always get cross nodes memory copy. And khugepaged may get slower if
cpu is contentious.

In addition, I saw MIPS has its own copy_user_highpage(), is it a
contributing factor too?

> +       }
>         new_page = khugepaged_alloc_page(hpage, gfp, node);
>         if (!new_page) {
>                 result = SCAN_ALLOC_HUGE_PAGE_FAIL;
> @@ -2380,6 +2389,7 @@ int start_stop_khugepaged(void)
>                 kthread_stop(khugepaged_thread);
>                 khugepaged_thread = NULL;
>         }
> +       khugepaged_scan.node = NUMA_NO_NODE;
>         set_recommended_min_free_kbytes();
>  fail:
>         mutex_unlock(&khugepaged_mutex);
> --
> 2.31.1
>
>

Re: [PATCH] mm/khugepaged: sched to numa node when collapse huge page

[maobibo]

Hi Yang,

thanks for reviewing my patch. I reply inline.

On 03/12/2022 03:08 AM, Yang Shi wrote:
> On Fri, Mar 11, 2022 at 1:01 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> collapse huge page is slow, specially when khugepaged daemon runs
>> on different numa node with that of huge page. It suffers from
>> huge page copying across nodes, also cache is not used for target
>> node. With this patch, khugepaged daemon switches to the same numa
>> node with huge page. It saves copying time and makes use of local
>> cache better.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>  mm/khugepaged.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 131492fd1148..460c285dc974 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -116,6 +116,7 @@ struct khugepaged_scan {
>>         struct list_head mm_head;
>>         struct mm_slot *mm_slot;
>>         unsigned long address;
>> +       int node;
>>  };
>>
>>  static struct khugepaged_scan khugepaged_scan = {
>> @@ -1066,6 +1067,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>>         struct vm_area_struct *vma;
>>         struct mmu_notifier_range range;
>>         gfp_t gfp;
>> +       const struct cpumask *cpumask;
>>
>>         VM_BUG_ON(address & ~HPAGE_PMD_MASK);
>>
>> @@ -1079,6 +1081,13 @@ static void collapse_huge_page(struct mm_struct *mm,
>>          * that. We will recheck the vma after taking it again in write mode.
>>          */
>>         mmap_read_unlock(mm);
>> +
>> +       /* sched to specified node before huage page memory copy */
>> +       cpumask = cpumask_of_node(node);
>> +       if ((khugepaged_scan.node != node) && !cpumask_empty(cpumask)) {
>> +               set_cpus_allowed_ptr(current, cpumask);
>> +               khugepaged_scan.node = node;
> 
> What if khugepaged was scheduled to the other nodes after this, but
> khugepaged_scan.node still equals to node? It seems possible to me
> IIUC.
khugepaged will not schedule to the other nodes after function 
set_cpus_allowed_ptr with my understanding. Or node is not necessary
to record and we can simply use task_node(current) like this:
 +       /* sched to specified node before huage page memory copy */
 +       cpumask = cpumask_of_node(node);
 +       if ((task_node(current) != node) && !cpumask_empty(cpumask))
 +               set_cpus_allowed_ptr(current, cpumask);
 
> 
> TBH I'm not quite sure if migrating khugepaged is really worth it for
> everyone or not. The worst case is the locality of base pages are not
> obvious, for example, the base pages may be across all nodes, so you
> always get cross nodes memory copy. And khugepaged may get slower if
> cpu is contentious.
target node is calculated from src node of base pages. If base pages
are across all nodes, target node is the most one from that of base pages.
Most time if THP is used, memory footprint for workload is large, it
is deserved for other architectures, however I have no such binary workload
on other architectures.

> 
> In addition, I saw MIPS has its own copy_user_highpage(), is it a
> contributing factor too?
copy_user_highpage is similar with function copy on mips64, it is only
different on mips32. It has nothing to do with this, else it will be big
issue.

regards
bibo,mao

> 
>> +       }
>>         new_page = khugepaged_alloc_page(hpage, gfp, node);
>>         if (!new_page) {
>>                 result = SCAN_ALLOC_HUGE_PAGE_FAIL;
>> @@ -2380,6 +2389,7 @@ int start_stop_khugepaged(void)
>>                 kthread_stop(khugepaged_thread);
>>                 khugepaged_thread = NULL;
>>         }
>> +       khugepaged_scan.node = NUMA_NO_NODE;
>>         set_recommended_min_free_kbytes();
>>  fail:
>>         mutex_unlock(&khugepaged_mutex);
>> --
>> 2.31.1
>>
>>