[PATCH 0/5] fork: Page operation cleanups in the fork code

[PATCH 0/5] fork: Page operation cleanups in the fork code

[Linus Walleij]

This patch set consists of outtakes from a 1 year+ old
patch set from Pasha, which all stand on their own.
See:
https://lore.kernel.org/all/20240311164638.2015063-1-pasha.tatashin@soleen.com/

What the code mainly does is make the fork.c file more
oriented around pages and remove reliance on THREAD_SIZE.

These are good cleanups for readability and in one case
(last patch using clear_page()) a performance improvement,
so I split these off, rebased on v6.15-rc1, addressed review
comments and send them separately.

All mentions of dynamic stack are removed from the patch
set as we have no idea whether that will go anywhere.

This is mostly MM related so when the patches are ready
I expect they would land in Andrew's patch stack.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Pasha Tatashin (5):
      fork: Clean-up ifdef logic around stack allocation
      fork: Clean-up naming of vm_strack/vm_struct variables in vmap stacks code
      fork: Remove assumption that vm_area->nr_pages equals to THREAD_SIZE
      fork: check charging success before zeroing stack
      fork: zero vmap stack using clear_page() instead of memset()

 kernel/fork.c | 99 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 48 insertions(+), 51 deletions(-)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250504-fork-fixes-9378b6c57873

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

[PATCH 1/5] fork: Clean-up ifdef logic around stack allocation

[Linus Walleij]

From: Pasha Tatashin <pasha.tatashin@soleen.com>

There is unneeded OR in the ifdef functions that are used to allocate
and free kernel stacks based on direct map or vmap.

Therefore, clean up by Changing the order so OR is no longer needed.

Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Link: https://lore.kernel.org/20240311164638.2015063-3-pasha.tatashin@soleen.com
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 kernel/fork.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index c4b26cd8998b8e7b2b516e0bb0b1d4676ff644dc..7b9e1ad141baaeb158b1807ea9fc3ef246f5f3a7 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -185,13 +185,7 @@ static inline void free_task_struct(struct task_struct *tsk)
 	kmem_cache_free(task_struct_cachep, tsk);
 }
 
-/*
- * Allocate pages if THREAD_SIZE is >= PAGE_SIZE, otherwise use a
- * kmemcache based allocator.
- */
-# if THREAD_SIZE >= PAGE_SIZE || defined(CONFIG_VMAP_STACK)
-
-#  ifdef CONFIG_VMAP_STACK
+#ifdef CONFIG_VMAP_STACK
 /*
  * vmalloc() is a bit slow, and calling vfree() enough times will force a TLB
  * flush.  Try to minimize the number of calls by caching stacks.
@@ -342,7 +336,13 @@ static void free_thread_stack(struct task_struct *tsk)
 	tsk->stack_vm_area = NULL;
 }
 
-#  else /* !CONFIG_VMAP_STACK */
+#else /* !CONFIG_VMAP_STACK */
+
+/*
+ * Allocate pages if THREAD_SIZE is >= PAGE_SIZE, otherwise use a
+ * kmemcache based allocator.
+ */
+#if THREAD_SIZE >= PAGE_SIZE
 
 static void thread_stack_free_rcu(struct rcu_head *rh)
 {
@@ -374,8 +374,7 @@ static void free_thread_stack(struct task_struct *tsk)
 	tsk->stack = NULL;
 }
 
-#  endif /* CONFIG_VMAP_STACK */
-# else /* !(THREAD_SIZE >= PAGE_SIZE || defined(CONFIG_VMAP_STACK)) */
+#else /* !(THREAD_SIZE >= PAGE_SIZE) */
 
 static struct kmem_cache *thread_stack_cache;
 
@@ -414,7 +413,8 @@ void thread_stack_cache_init(void)
 	BUG_ON(thread_stack_cache == NULL);
 }
 
-# endif /* THREAD_SIZE >= PAGE_SIZE || defined(CONFIG_VMAP_STACK) */
+#endif /* THREAD_SIZE >= PAGE_SIZE */
+#endif /* CONFIG_VMAP_STACK */
 
 /* SLAB cache for signal_struct structures (tsk->signal) */
 static struct kmem_cache *signal_cachep;

-- 
2.49.0

[PATCH 2/5] fork: Clean-up naming of vm_strack/vm_struct variables in vmap stacks code

[Linus Walleij]

From: Pasha Tatashin <pasha.tatashin@soleen.com>

There are two data types: "struct vm_struct" and "struct vm_stack" that
have the same local variable names: vm_stack, or vm, or s, which makes
the code confusing to read.

Change the code so the naming is consistent:

struct vm_struct is always called vm_area
struct vm_stack is always called vm_stack

One change altering vfree(vm_stack) to vfree(vm_area->addr) may look
like a semantic change but it is not: vm_area->addr points to the
vm_stack. This was done to improve readability.

Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Link: https://lore.kernel.org/20240311164638.2015063-4-pasha.tatashin@soleen.com
[Rebased and added new users of the variable names, address review comments]
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 kernel/fork.c | 58 ++++++++++++++++++++++++++++------------------------------
 1 file changed, 28 insertions(+), 30 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 7b9e1ad141baaeb158b1807ea9fc3ef246f5f3a7..3bed36a56292e3b868895dac169266c428f8c1ff 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -198,14 +198,14 @@ struct vm_stack {
 	struct vm_struct *stack_vm_area;
 };
 
-static bool try_release_thread_stack_to_cache(struct vm_struct *vm)
+static bool try_release_thread_stack_to_cache(struct vm_struct *vm_area)
 {
 	unsigned int i;
 
 	for (i = 0; i < NR_CACHED_STACKS; i++) {
 		struct vm_struct *tmp = NULL;
 
-		if (this_cpu_try_cmpxchg(cached_stacks[i], &tmp, vm))
+		if (this_cpu_try_cmpxchg(cached_stacks[i], &tmp, vm_area))
 			return true;
 	}
 	return false;
@@ -214,11 +214,12 @@ static bool try_release_thread_stack_to_cache(struct vm_struct *vm)
 static void thread_stack_free_rcu(struct rcu_head *rh)
 {
 	struct vm_stack *vm_stack = container_of(rh, struct vm_stack, rcu);
+	struct vm_struct *vm_area = vm_stack->stack_vm_area;
 
 	if (try_release_thread_stack_to_cache(vm_stack->stack_vm_area))
 		return;
 
-	vfree(vm_stack);
+	vfree(vm_area->addr);
 }
 
 static void thread_stack_delayed_free(struct task_struct *tsk)
@@ -231,23 +232,23 @@ static void thread_stack_delayed_free(struct task_struct *tsk)
 
 static int free_vm_stack_cache(unsigned int cpu)
 {
-	struct vm_struct **cached_vm_stacks = per_cpu_ptr(cached_stacks, cpu);
+	struct vm_struct **cached_vm_stack_areas = per_cpu_ptr(cached_stacks, cpu);
 	int i;
 
 	for (i = 0; i < NR_CACHED_STACKS; i++) {
-		struct vm_struct *vm_stack = cached_vm_stacks[i];
+		struct vm_struct *vm_area = cached_vm_stack_areas[i];
 
-		if (!vm_stack)
+		if (!vm_area)
 			continue;
 
-		vfree(vm_stack->addr);
-		cached_vm_stacks[i] = NULL;
+		vfree(vm_area->addr);
+		cached_vm_stack_areas[i] = NULL;
 	}
 
 	return 0;
 }
 
-static int memcg_charge_kernel_stack(struct vm_struct *vm)
+static int memcg_charge_kernel_stack(struct vm_struct *vm_area)
 {
 	int i;
 	int ret;
@@ -256,7 +257,7 @@ static int memcg_charge_kernel_stack(struct vm_struct *vm)
 	BUG_ON(vm->nr_pages != THREAD_SIZE / PAGE_SIZE);
 
 	for (i = 0; i < THREAD_SIZE / PAGE_SIZE; i++) {
-		ret = memcg_kmem_charge_page(vm->pages[i], GFP_KERNEL, 0);
+		ret = memcg_kmem_charge_page(vm_area->pages[i], GFP_KERNEL, 0);
 		if (ret)
 			goto err;
 		nr_charged++;
@@ -264,38 +265,35 @@ static int memcg_charge_kernel_stack(struct vm_struct *vm)
 	return 0;
 err:
 	for (i = 0; i < nr_charged; i++)
-		memcg_kmem_uncharge_page(vm->pages[i], 0);
+		memcg_kmem_uncharge_page(vm_area->pages[i], 0);
 	return ret;
 }
 
 static int alloc_thread_stack_node(struct task_struct *tsk, int node)
 {
-	struct vm_struct *vm;
+	struct vm_struct *vm_area;
 	void *stack;
 	int i;
 
 	for (i = 0; i < NR_CACHED_STACKS; i++) {
-		struct vm_struct *s;
-
-		s = this_cpu_xchg(cached_stacks[i], NULL);
-
-		if (!s)
+		vm_area = this_cpu_xchg(cached_stacks[i], NULL);
+		if (!vm_area)
 			continue;
 
 		/* Reset stack metadata. */
-		kasan_unpoison_range(s->addr, THREAD_SIZE);
+		kasan_unpoison_range(vm_area->addr, THREAD_SIZE);
 
-		stack = kasan_reset_tag(s->addr);
+		stack = kasan_reset_tag(vm_area->addr);
 
 		/* Clear stale pointers from reused stack. */
 		memset(stack, 0, THREAD_SIZE);
 
-		if (memcg_charge_kernel_stack(s)) {
-			vfree(s->addr);
+		if (memcg_charge_kernel_stack(vm_area)) {
+			vfree(vm_area->addr);
 			return -ENOMEM;
 		}
 
-		tsk->stack_vm_area = s;
+		tsk->stack_vm_area = vm_area;
 		tsk->stack = stack;
 		return 0;
 	}
@@ -311,8 +309,8 @@ static int alloc_thread_stack_node(struct task_struct *tsk, int node)
 	if (!stack)
 		return -ENOMEM;
 
-	vm = find_vm_area(stack);
-	if (memcg_charge_kernel_stack(vm)) {
+	vm_area = find_vm_area(stack);
+	if (memcg_charge_kernel_stack(vm_area)) {
 		vfree(stack);
 		return -ENOMEM;
 	}
@@ -321,7 +319,7 @@ static int alloc_thread_stack_node(struct task_struct *tsk, int node)
 	 * free_thread_stack() can be called in interrupt context,
 	 * so cache the vm_struct.
 	 */
-	tsk->stack_vm_area = vm;
+	tsk->stack_vm_area = vm_area;
 	stack = kasan_reset_tag(stack);
 	tsk->stack = stack;
 	return 0;
@@ -517,11 +515,11 @@ void vm_area_free(struct vm_area_struct *vma)
 static void account_kernel_stack(struct task_struct *tsk, int account)
 {
 	if (IS_ENABLED(CONFIG_VMAP_STACK)) {
-		struct vm_struct *vm = task_stack_vm_area(tsk);
+		struct vm_struct *vm_area = task_stack_vm_area(tsk);
 		int i;
 
 		for (i = 0; i < THREAD_SIZE / PAGE_SIZE; i++)
-			mod_lruvec_page_state(vm->pages[i], NR_KERNEL_STACK_KB,
+			mod_lruvec_page_state(vm_area->pages[i], NR_KERNEL_STACK_KB,
 					      account * (PAGE_SIZE / 1024));
 	} else {
 		void *stack = task_stack_page(tsk);
@@ -537,12 +535,12 @@ void exit_task_stack_account(struct task_struct *tsk)
 	account_kernel_stack(tsk, -1);
 
 	if (IS_ENABLED(CONFIG_VMAP_STACK)) {
-		struct vm_struct *vm;
+		struct vm_struct *vm_area;
 		int i;
 
-		vm = task_stack_vm_area(tsk);
+		vm_area = task_stack_vm_area(tsk);
 		for (i = 0; i < THREAD_SIZE / PAGE_SIZE; i++)
-			memcg_kmem_uncharge_page(vm->pages[i], 0);
+			memcg_kmem_uncharge_page(vm_area->pages[i], 0);
 	}
 }
 

-- 
2.49.0

[PATCH 3/5] fork: Remove assumption that vm_area->nr_pages equals to THREAD_SIZE

[Linus Walleij]

From: Pasha Tatashin <pasha.tatashin@soleen.com>

In many places number of pages in the stack is detremined via
(THREAD_SIZE / PAGE_SIZE). There is also a BUG_ON() that ensures that
(THREAD_SIZE / PAGE_SIZE) is indeed equal to vm_area->nr_pages.

Consistently use vm_area->nr_pages to determine the actual number
of pages allocated in the stack instead, so it is clear what is
going on here.

Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Link: https://lore.kernel.org/20240311164638.2015063-5-pasha.tatashin@soleen.com
[Rebased, also skipped intermediary helper variable nr_pages]
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 kernel/fork.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 3bed36a56292e3b868895dac169266c428f8c1ff..02943c14ee3b932db11cd9292051fdf5ce381a38 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -254,9 +254,7 @@ static int memcg_charge_kernel_stack(struct vm_struct *vm_area)
 	int ret;
 	int nr_charged = 0;
 
-	BUG_ON(vm->nr_pages != THREAD_SIZE / PAGE_SIZE);
-
-	for (i = 0; i < THREAD_SIZE / PAGE_SIZE; i++) {
+	for (i = 0; i < vm_area->nr_pages; i++) {
 		ret = memcg_kmem_charge_page(vm_area->pages[i], GFP_KERNEL, 0);
 		if (ret)
 			goto err;
@@ -518,7 +516,7 @@ static void account_kernel_stack(struct task_struct *tsk, int account)
 		struct vm_struct *vm_area = task_stack_vm_area(tsk);
 		int i;
 
-		for (i = 0; i < THREAD_SIZE / PAGE_SIZE; i++)
+		for (i = 0; i < vm_area->nr_pages; i++)
 			mod_lruvec_page_state(vm_area->pages[i], NR_KERNEL_STACK_KB,
 					      account * (PAGE_SIZE / 1024));
 	} else {
@@ -539,7 +537,7 @@ void exit_task_stack_account(struct task_struct *tsk)
 		int i;
 
 		vm_area = task_stack_vm_area(tsk);
-		for (i = 0; i < THREAD_SIZE / PAGE_SIZE; i++)
+		for (i = 0; i < vm_area->nr_pages; i++)
 			memcg_kmem_uncharge_page(vm_area->pages[i], 0);
 	}
 }

-- 
2.49.0

[PATCH 4/5] fork: check charging success before zeroing stack

[Linus Walleij]

From: Pasha Tatashin <pasha.tatashin@soleen.com>

No need to do zero cached stack if memcg charge fails, so move the
charging attempt before the memset operation.

Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Link: https://lore.kernel.org/20240311164638.2015063-6-pasha.tatashin@soleen.com
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 kernel/fork.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 02943c14ee3b932db11cd9292051fdf5ce381a38..2350ba55db2a5f5675f810d7214252dd8a1fae98 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -278,6 +278,11 @@ static int alloc_thread_stack_node(struct task_struct *tsk, int node)
 		if (!vm_area)
 			continue;
 
+		if (memcg_charge_kernel_stack(vm_area)) {
+			vfree(vm_area->addr);
+			return -ENOMEM;
+		}
+
 		/* Reset stack metadata. */
 		kasan_unpoison_range(vm_area->addr, THREAD_SIZE);
 
@@ -286,11 +291,6 @@ static int alloc_thread_stack_node(struct task_struct *tsk, int node)
 		/* Clear stale pointers from reused stack. */
 		memset(stack, 0, THREAD_SIZE);
 
-		if (memcg_charge_kernel_stack(vm_area)) {
-			vfree(vm_area->addr);
-			return -ENOMEM;
-		}
-
 		tsk->stack_vm_area = vm_area;
 		tsk->stack = stack;
 		return 0;

-- 
2.49.0

[PATCH 5/5] fork: zero vmap stack using clear_page() instead of memset()

[Linus Walleij]

From: Pasha Tatashin <pasha.tatashin@soleen.com>

Do not zero the whole span of the stack, but instead only the
pages that are part of the vm_area.

As several architectures have optimized implementations of
clear_page(), this will give the architecture a clear idea
of what is going on and will speed up the clearing operation.

Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Link: https://lore.kernel.org/20240311164638.2015063-7-pasha.tatashin@soleen.com
[Rebased, also skipped intermediary helper variable nr_pages]
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 kernel/fork.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 2350ba55db2a5f5675f810d7214252dd8a1fae98..7710b7ded3000c77e8d0871bb6bb5916d90aa4ec 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -271,7 +271,7 @@ static int alloc_thread_stack_node(struct task_struct *tsk, int node)
 {
 	struct vm_struct *vm_area;
 	void *stack;
-	int i;
+	int i, j;
 
 	for (i = 0; i < NR_CACHED_STACKS; i++) {
 		vm_area = this_cpu_xchg(cached_stacks[i], NULL);
@@ -289,7 +289,8 @@ static int alloc_thread_stack_node(struct task_struct *tsk, int node)
 		stack = kasan_reset_tag(vm_area->addr);
 
 		/* Clear stale pointers from reused stack. */
-		memset(stack, 0, THREAD_SIZE);
+		for (j = 0; j < vm_area->nr_pages; j++)
+			clear_page(page_address(vm_area->pages[j]));
 
 		tsk->stack_vm_area = vm_area;
 		tsk->stack = stack;

-- 
2.49.0

Re: [PATCH 2/5] fork: Clean-up naming of vm_strack/vm_struct variables in vmap stacks code

[Andrew Morton]

On Tue, 06 May 2025 09:57:54 +0200 Linus Walleij <linus.walleij@linaro.org> wrote:

> Subject: [PATCH 2/5] fork: Clean-up naming of vm_strack/vm_struct variables in vmap stacks code

s/strack/stack/

> Date: Tue, 06 May 2025 09:57:54 +0200
> X-Mailer: b4 0.14.2
> 
> From: Pasha Tatashin <pasha.tatashin@soleen.com>
> 
> There are two data types: "struct vm_struct" and "struct vm_stack" that
> have the same local variable names: vm_stack, or vm, or s, which makes
> the code confusing to read.
> 
> Change the code so the naming is consistent:
> 
> struct vm_struct is always called vm_area
> struct vm_stack is always called vm_stack
> 
> One change altering vfree(vm_stack) to vfree(vm_area->addr) may look
> like a semantic change but it is not: vm_area->addr points to the
> vm_stack. This was done to improve readability.

This breaks the build

In file included from ./include/linux/export.h:5,
                 from ./include/linux/linkage.h:7,
                 from ./arch/x86/include/asm/cache.h:5,
                 from ./include/vdso/cache.h:5,
                 from ./include/linux/cache.h:6,
                 from ./include/linux/slab.h:15,
                 from kernel/fork.c:16:
kernel/fork.c: In function 'memcg_charge_kernel_stack':
kernel/fork.c:260:16: error: 'vm' undeclared (first use in this function); did you mean 'tm'?
  260 |         BUG_ON(vm->nr_pages != THREAD_SIZE / PAGE_SIZE);
      |                ^~
./include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
   77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
      |                                             ^
kernel/fork.c:260:9: note: in expansion of macro 'BUG_ON'
  260 |         BUG_ON(vm->nr_pages != THREAD_SIZE / PAGE_SIZE);
      |         ^~~~~~
kernel/fork.c:260:16: note: each undeclared identifier is reported only once for each function it appears in
  260 |         BUG_ON(vm->nr_pages != THREAD_SIZE / PAGE_SIZE);
      |                ^~
./include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
   77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
      |                                             ^
kernel/fork.c:260:9: note: in expansion of macro 'BUG_ON'
  260 |         BUG_ON(vm->nr_pages != THREAD_SIZE / PAGE_SIZE);
      |         ^~~~~~

The error gets fixed up in the next patch.  Not a huge problem but this
can disrupt git-bisect operations.

Re: [PATCH 3/5] fork: Remove assumption that vm_area->nr_pages equals to THREAD_SIZE

[Andrew Morton]

On Tue, 06 May 2025 09:57:55 +0200 Linus Walleij <linus.walleij@linaro.org> wrote:

> From: Pasha Tatashin <pasha.tatashin@soleen.com>
> 
> In many places number of pages in the stack is detremined via
> (THREAD_SIZE / PAGE_SIZE). There is also a BUG_ON() that ensures that
> (THREAD_SIZE / PAGE_SIZE) is indeed equal to vm_area->nr_pages.
> 
> Consistently use vm_area->nr_pages to determine the actual number
> of pages allocated in the stack instead, so it is clear what is
> going on here.
> 
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> Link: https://lore.kernel.org/20240311164638.2015063-5-pasha.tatashin@soleen.com
> [Rebased, also skipped intermediary helper variable nr_pages]

I like

[linus.walleij@linaro.org: rebased, also skipped intermediary helper variable nr_pages]

> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -254,9 +254,7 @@ static int memcg_charge_kernel_stack(struct vm_struct *vm_area)
>  	int ret;
>  	int nr_charged = 0;
>  
> -	BUG_ON(vm->nr_pages != THREAD_SIZE / PAGE_SIZE);
> -
> -	for (i = 0; i < THREAD_SIZE / PAGE_SIZE; i++) {
> +	for (i = 0; i < vm_area->nr_pages; i++) {

Well, constants are fast and small.  I assume this will add a few
instructions and a bit of runtime overhead?

>  		ret = memcg_kmem_charge_page(vm_area->pages[i], GFP_KERNEL, 0);
>  		if (ret)
>  			goto err;
> @@ -518,7 +516,7 @@ static void account_kernel_stack(struct task_struct *tsk, int account)
>  		struct vm_struct *vm_area = task_stack_vm_area(tsk);
>  		int i;
>  
> -		for (i = 0; i < THREAD_SIZE / PAGE_SIZE; i++)
> +		for (i = 0; i < vm_area->nr_pages; i++)
>  			mod_lruvec_page_state(vm_area->pages[i], NR_KERNEL_STACK_KB,
>  					      account * (PAGE_SIZE / 1024));
>  	} else {

Re: [PATCH 3/5] fork: Remove assumption that vm_area->nr_pages equals to THREAD_SIZE

[Linus Walleij]

On Wed, May 7, 2025 at 3:53 AM Andrew Morton <akpm@linux-foundation.org> wrote:

> > -     BUG_ON(vm->nr_pages != THREAD_SIZE / PAGE_SIZE);
> > -
> > -     for (i = 0; i < THREAD_SIZE / PAGE_SIZE; i++) {
> > +     for (i = 0; i < vm_area->nr_pages; i++) {
>
> Well, constants are fast and small.  I assume this will add a few
> instructions and a bit of runtime overhead?

That is true.

Pashas original patch has a local variable instead of comparing
vm_area->nr_pages, so I went back to that to ascertain we
are not dereferencing the struct on every iteration of the
loop, but will instead use a register.

For this first instance the assignment of that variable should be no
more time consuming than the deleted BUG_ON() dereference so
it is +/- 0.

For the other two instances I like the readability this gives, the
local variable restored there too.

Maybe those two should have also has BUG_ON() guards
actually. vm_area->nr_pages should be the trusted, run-time
value.

I'll resend what I have so you can decide.

Yours,
Linus Walleij