[PATCH 1/1] mm/mmu_gather: limit tlb batch count and add schedule point in tlb_batch_pages_flush

[PATCH 1/1] mm/mmu_gather: limit tlb batch count and add schedule point in tlb_batch_pages_flush

[Jianxing Wang]

free a large list of pages maybe cause rcu_sched starved on
non-preemptible kernels. howerver free_unref_page_list maybe can't
cond_resched as it maybe called in interrupt or atomic context,
especially can't detect atomic context in CONFIG_PREEMPTION=n.

tlb flush batch count depends on PAGE_SIZE, it's too large if
PAGE_SIZE > 4K, here limit max batch size with 4K.
And add schedule point in tlb_batch_pages_flush.

rcu: rcu_sched kthread starved for 5359 jiffies! g454793 f0x0
RCU_GP_WAIT_FQS(5) ->state=0x0 ->cpu=19
[...]
Call Trace:
   free_unref_page_list+0x19c/0x270
   release_pages+0x3cc/0x498
   tlb_flush_mmu_free+0x44/0x70
   zap_pte_range+0x450/0x738
   unmap_page_range+0x108/0x240
   unmap_vmas+0x74/0xf0
   unmap_region+0xb0/0x120
   do_munmap+0x264/0x438
   vm_munmap+0x58/0xa0
   sys_munmap+0x10/0x20
   syscall_common+0x24/0x38

Signed-off-by: Jianxing Wang <wangjianxing@loongson.cn>
---
 include/asm-generic/tlb.h | 7 ++++++-
 mm/mmu_gather.c           | 7 +++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 2c68a545ffa7..47c7f93ca695 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -230,8 +230,13 @@ struct mmu_gather_batch {
 	struct page		*pages[0];
 };
 
+#if PAGE_SIZE > 4096UL
+#define MAX_GATHER_BATCH_SZ	4096
+#else
+#define MAX_GATHER_BATCH_SZ	PAGE_SIZE
+#endif
 #define MAX_GATHER_BATCH	\
-	((PAGE_SIZE - sizeof(struct mmu_gather_batch)) / sizeof(void *))
+	((MAX_GATHER_BATCH_SZ - sizeof(struct mmu_gather_batch)) / sizeof(void *))
 
 /*
  * Limit the maximum number of mmu_gather batches to reduce a risk of soft
diff --git a/mm/mmu_gather.c b/mm/mmu_gather.c
index afb7185ffdc4..f2c105810b3f 100644
--- a/mm/mmu_gather.c
+++ b/mm/mmu_gather.c
@@ -8,6 +8,7 @@
 #include <linux/rcupdate.h>
 #include <linux/smp.h>
 #include <linux/swap.h>
+#include <linux/slab.h>
 
 #include <asm/pgalloc.h>
 #include <asm/tlb.h>
@@ -27,7 +28,7 @@ static bool tlb_next_batch(struct mmu_gather *tlb)
 	if (tlb->batch_count == MAX_GATHER_BATCH_COUNT)
 		return false;
 
-	batch = (void *)__get_free_pages(GFP_NOWAIT | __GFP_NOWARN, 0);
+	batch = kmalloc(MAX_GATHER_BATCH_SZ, GFP_NOWAIT | __GFP_NOWARN);
 	if (!batch)
 		return false;
 
@@ -49,6 +50,8 @@ static void tlb_batch_pages_flush(struct mmu_gather *tlb)
 	for (batch = &tlb->local; batch && batch->nr; batch = batch->next) {
 		free_pages_and_swap_cache(batch->pages, batch->nr);
 		batch->nr = 0;
+
+		cond_resched();
 	}
 	tlb->active = &tlb->local;
 }
@@ -59,7 +62,7 @@ static void tlb_batch_list_free(struct mmu_gather *tlb)
 
 	for (batch = tlb->local.next; batch; batch = next) {
 		next = batch->next;
-		free_pages((unsigned long)batch, 0);
+		kfree(batch);
 	}
 	tlb->local.next = NULL;
 }
-- 
2.31.1

Re: [PATCH 1/1] mm/mmu_gather: limit tlb batch count and add schedule point in tlb_batch_pages_flush

[wangjianxing]

1. I try to increase the overcommit ratio of cpu to 1:2~1:3 in KVM
hypervisor, per-vm has the same number of vcpu with host cpu, then setup
2 or 3 vm.

2. Run ltpstress(20180926) test in per vm, both host and guest is 
non-preemptiable
kernel, vm dmesg will throw some rcu_sched warning.

3. PAGE_SIZE in my kernel config is 16K, tlb batch max count is 2015, 
it's too long in non-preemptible state.

The issue's orignal 
link:https://patchwork.kernel.org/project/linux-mm/patch/20220302013825.2290315-1-wangjianxing@loongson.cn/


On 03/15/2022 08:55 PM, Jianxing Wang wrote:
> free a large list of pages maybe cause rcu_sched starved on
> non-preemptible kernels. howerver free_unref_page_list maybe can't
> cond_resched as it maybe called in interrupt or atomic context,
> especially can't detect atomic context in CONFIG_PREEMPTION=n.
>
> tlb flush batch count depends on PAGE_SIZE, it's too large if
> PAGE_SIZE > 4K, here limit max batch size with 4K.
> And add schedule point in tlb_batch_pages_flush.
>
> rcu: rcu_sched kthread starved for 5359 jiffies! g454793 f0x0
> RCU_GP_WAIT_FQS(5) ->state=0x0 ->cpu=19
> [...]
> Call Trace:
>     free_unref_page_list+0x19c/0x270
>     release_pages+0x3cc/0x498
>     tlb_flush_mmu_free+0x44/0x70
>     zap_pte_range+0x450/0x738
>     unmap_page_range+0x108/0x240
>     unmap_vmas+0x74/0xf0
>     unmap_region+0xb0/0x120
>     do_munmap+0x264/0x438
>     vm_munmap+0x58/0xa0
>     sys_munmap+0x10/0x20
>     syscall_common+0x24/0x38
>
> Signed-off-by: Jianxing Wang <wangjianxing@loongson.cn>
> ---
>   include/asm-generic/tlb.h | 7 ++++++-
>   mm/mmu_gather.c           | 7 +++++--
>   2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
> index 2c68a545ffa7..47c7f93ca695 100644
> --- a/include/asm-generic/tlb.h
> +++ b/include/asm-generic/tlb.h
> @@ -230,8 +230,13 @@ struct mmu_gather_batch {
>   	struct page		*pages[0];
>   };
>   
> +#if PAGE_SIZE > 4096UL
> +#define MAX_GATHER_BATCH_SZ	4096
> +#else
> +#define MAX_GATHER_BATCH_SZ	PAGE_SIZE
> +#endif
>   #define MAX_GATHER_BATCH	\
> -	((PAGE_SIZE - sizeof(struct mmu_gather_batch)) / sizeof(void *))
> +	((MAX_GATHER_BATCH_SZ - sizeof(struct mmu_gather_batch)) / sizeof(void *))
>   
>   /*
>    * Limit the maximum number of mmu_gather batches to reduce a risk of soft
> diff --git a/mm/mmu_gather.c b/mm/mmu_gather.c
> index afb7185ffdc4..f2c105810b3f 100644
> --- a/mm/mmu_gather.c
> +++ b/mm/mmu_gather.c
> @@ -8,6 +8,7 @@
>   #include <linux/rcupdate.h>
>   #include <linux/smp.h>
>   #include <linux/swap.h>
> +#include <linux/slab.h>
>   
>   #include <asm/pgalloc.h>
>   #include <asm/tlb.h>
> @@ -27,7 +28,7 @@ static bool tlb_next_batch(struct mmu_gather *tlb)
>   	if (tlb->batch_count == MAX_GATHER_BATCH_COUNT)
>   		return false;
>   
> -	batch = (void *)__get_free_pages(GFP_NOWAIT | __GFP_NOWARN, 0);
> +	batch = kmalloc(MAX_GATHER_BATCH_SZ, GFP_NOWAIT | __GFP_NOWARN);
>   	if (!batch)
>   		return false;
>   
> @@ -49,6 +50,8 @@ static void tlb_batch_pages_flush(struct mmu_gather *tlb)
>   	for (batch = &tlb->local; batch && batch->nr; batch = batch->next) {
>   		free_pages_and_swap_cache(batch->pages, batch->nr);
>   		batch->nr = 0;
> +
> +		cond_resched();
>   	}
>   	tlb->active = &tlb->local;
>   }
> @@ -59,7 +62,7 @@ static void tlb_batch_list_free(struct mmu_gather *tlb)
>   
>   	for (batch = tlb->local.next; batch; batch = next) {
>   		next = batch->next;
> -		free_pages((unsigned long)batch, 0);
> +		kfree(batch);
>   	}
>   	tlb->local.next = NULL;
>   }

Re: [PATCH 1/1] mm/mmu_gather: limit tlb batch count and add schedule point in tlb_batch_pages_flush

[Peter Zijlstra]

On Tue, Mar 15, 2022 at 08:55:36AM -0400, Jianxing Wang wrote:
> free a large list of pages maybe cause rcu_sched starved on
> non-preemptible kernels. howerver free_unref_page_list maybe can't
> cond_resched as it maybe called in interrupt or atomic context,
> especially can't detect atomic context in CONFIG_PREEMPTION=n.
> 
> tlb flush batch count depends on PAGE_SIZE, it's too large if
> PAGE_SIZE > 4K, here limit max batch size with 4K.
> And add schedule point in tlb_batch_pages_flush.
> 
> rcu: rcu_sched kthread starved for 5359 jiffies! g454793 f0x0
> RCU_GP_WAIT_FQS(5) ->state=0x0 ->cpu=19
> [...]
> Call Trace:
>    free_unref_page_list+0x19c/0x270
>    release_pages+0x3cc/0x498
>    tlb_flush_mmu_free+0x44/0x70
>    zap_pte_range+0x450/0x738
>    unmap_page_range+0x108/0x240
>    unmap_vmas+0x74/0xf0
>    unmap_region+0xb0/0x120
>    do_munmap+0x264/0x438
>    vm_munmap+0x58/0xa0
>    sys_munmap+0x10/0x20
>    syscall_common+0x24/0x38
> 
> Signed-off-by: Jianxing Wang <wangjianxing@loongson.cn>
> ---
>  include/asm-generic/tlb.h | 7 ++++++-
>  mm/mmu_gather.c           | 7 +++++--
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
> index 2c68a545ffa7..47c7f93ca695 100644
> --- a/include/asm-generic/tlb.h
> +++ b/include/asm-generic/tlb.h
> @@ -230,8 +230,13 @@ struct mmu_gather_batch {
>  	struct page		*pages[0];
>  };
>  
> +#if PAGE_SIZE > 4096UL
> +#define MAX_GATHER_BATCH_SZ	4096
> +#else
> +#define MAX_GATHER_BATCH_SZ	PAGE_SIZE
> +#endif
>  #define MAX_GATHER_BATCH	\
> -	((PAGE_SIZE - sizeof(struct mmu_gather_batch)) / sizeof(void *))
> +	((MAX_GATHER_BATCH_SZ - sizeof(struct mmu_gather_batch)) / sizeof(void *))
>  
>  /*
>   * Limit the maximum number of mmu_gather batches to reduce a risk of soft
> diff --git a/mm/mmu_gather.c b/mm/mmu_gather.c
> index afb7185ffdc4..f2c105810b3f 100644
> --- a/mm/mmu_gather.c
> +++ b/mm/mmu_gather.c
> @@ -8,6 +8,7 @@
>  #include <linux/rcupdate.h>
>  #include <linux/smp.h>
>  #include <linux/swap.h>
> +#include <linux/slab.h>
>  
>  #include <asm/pgalloc.h>
>  #include <asm/tlb.h>
> @@ -27,7 +28,7 @@ static bool tlb_next_batch(struct mmu_gather *tlb)
>  	if (tlb->batch_count == MAX_GATHER_BATCH_COUNT)
>  		return false;
>  
> -	batch = (void *)__get_free_pages(GFP_NOWAIT | __GFP_NOWARN, 0);
> +	batch = kmalloc(MAX_GATHER_BATCH_SZ, GFP_NOWAIT | __GFP_NOWARN);
>  	if (!batch)
>  		return false;
>  
> @@ -49,6 +50,8 @@ static void tlb_batch_pages_flush(struct mmu_gather *tlb)
>  	for (batch = &tlb->local; batch && batch->nr; batch = batch->next) {
>  		free_pages_and_swap_cache(batch->pages, batch->nr);
>  		batch->nr = 0;
> +
> +		cond_resched();
>  	}
>  	tlb->active = &tlb->local;
>  }
> @@ -59,7 +62,7 @@ static void tlb_batch_list_free(struct mmu_gather *tlb)
>  
>  	for (batch = tlb->local.next; batch; batch = next) {
>  		next = batch->next;
> -		free_pages((unsigned long)batch, 0);
> +		kfree(batch);
>  	}
>  	tlb->local.next = NULL;
>  }

This seems like a really complicated way of writing something like the
below...

diff --git a/mm/mmu_gather.c b/mm/mmu_gather.c
index afb7185ffdc4..b382e86c1b47 100644
--- a/mm/mmu_gather.c
+++ b/mm/mmu_gather.c
@@ -47,8 +47,17 @@ static void tlb_batch_pages_flush(struct mmu_gather *tlb)
 	struct mmu_gather_batch *batch;
 
 	for (batch = &tlb->local; batch && batch->nr; batch = batch->next) {
-		free_pages_and_swap_cache(batch->pages, batch->nr);
-		batch->nr = 0;
+		struct page_struct *pages = batch->pages;
+
+		do {
+			int nr = min(512, batch->nr);
+
+			free_pages_and_swap_cache(pages, nr);
+			pages += nr;
+			batch->nr -= nr;
+
+			cond_resched();
+		} while (batch->nr);
 	}
 	tlb->active = &tlb->local;
 }

Re: [PATCH 1/1] mm/mmu_gather: limit tlb batch count and add schedule point in tlb_batch_pages_flush

[wangjianxing]

On 03/16/2022 04:57 PM, Peter Zijlstra wrote:
> This seems like a really complicated way of writing something like the
> below...
>
> diff --git a/mm/mmu_gather.c b/mm/mmu_gather.c
> index afb7185ffdc4..b382e86c1b47 100644
> --- a/mm/mmu_gather.c
> +++ b/mm/mmu_gather.c
> @@ -47,8 +47,17 @@ static void tlb_batch_pages_flush(struct mmu_gather *tlb)
>   	struct mmu_gather_batch *batch;
>   
>   	for (batch = &tlb->local; batch && batch->nr; batch = batch->next) {
> -		free_pages_and_swap_cache(batch->pages, batch->nr);
> -		batch->nr = 0;
> +		struct page_struct *pages = batch->pages;
> +
> +		do {
> +			int nr = min(512, batch->nr);
> +
> +			free_pages_and_swap_cache(pages, nr);
> +			pages += nr;
> +			batch->nr -= nr;
> +
> +			cond_resched();
> +		} while (batch->nr);
>   	}
>   	tlb->active = &tlb->local;
>   }
Yeah, it looks nicer.

I will resubmit the patch.