linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fork: define a local GFP_VMAP_STACK
@ 2025-05-09  7:25 Linus Walleij
  0 siblings, 0 replies; only message in thread
From: Linus Walleij @ 2025-05-09  7:25 UTC (permalink / raw)
  To: Andrew Morton, linux-mm, Pasha Tatashin, Mateusz Guzik; +Cc: Linus Walleij

The current allocation of VMAP stack memory is using
(THREADINFO_GFP & ~__GFP_ACCOUNT) which is a complicated
way of saying (GFP_KERNEL | __GFP_ZERO):

<linux/thread_info.h>:
define THREADINFO_GFP (GFP_KERNEL_ACCOUNT | __GFP_ZERO)
<linux/gfp_types.h>:
define GFP_KERNEL_ACCOUNT (GFP_KERNEL | __GFP_ACCOUNT)

This is an unfortunate side-effect of independent
changes blurring the picture:

commit 19809c2da28aee5860ad9a2eff760730a0710df0
changed (THREADINFO_GFP | __GFP_HIGHMEM) to just
THREADINFO_GFP since highmem became implicit.

commit 9b6f7e163cd0f468d1b9696b785659d3c27c8667
then added stack caching and rewrote the allocation to
(THREADINFO_GFP & ~__GFP_ACCOUNT) as cached stacks
need to be accounted separately. However that code,
when it eventually accounts the memory does this:

  ret = memcg_kmem_charge(vm->pages[i], GFP_KERNEL, 0)

so the memory is charged as a GFP_KERNEL allocation.

Define a unique GFP_VMAP_STACK to use
GFP_KERNEL | __GFP_ZERO and move the comment there.

Reported-by: Mateusz Guzik <mjguzik@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 kernel/fork.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index c4b26cd8998b8e7b2b516e0bb0b1d4676ff644dc..bd950104fca925a4bb4f7d220f00ef8997ddf4e2 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -198,6 +198,12 @@ static inline void free_task_struct(struct task_struct *tsk)
  */
 #define NR_CACHED_STACKS 2
 static DEFINE_PER_CPU(struct vm_struct *, cached_stacks[NR_CACHED_STACKS]);
+/*
+ * Allocated stacks are cached and later reused by new threads, so memcg
+ * accounting is performed by the code assigning/releasing stacks to tasks.
+ * We need a zeroed memory without __GFP_ACCOUNT.
+ */
+#define GFP_VMAP_STACK (GFP_KERNEL | __GFP_ZERO)
 
 struct vm_stack {
 	struct rcu_head rcu;
@@ -306,13 +312,8 @@ static int alloc_thread_stack_node(struct task_struct *tsk, int node)
 		return 0;
 	}
 
-	/*
-	 * Allocated stacks are cached and later reused by new threads,
-	 * so memcg accounting is performed manually on assigning/releasing
-	 * stacks to tasks. Drop __GFP_ACCOUNT.
-	 */
 	stack = __vmalloc_node(THREAD_SIZE, THREAD_ALIGN,
-				     THREADINFO_GFP & ~__GFP_ACCOUNT,
+				     GFP_VMAP_STACK,
 				     node, __builtin_return_address(0));
 	if (!stack)
 		return -ENOMEM;

---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250509-gfp-stack-3750ed20fe9d

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



^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2025-05-09  7:25 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-09  7:25 [PATCH] fork: define a local GFP_VMAP_STACK Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox