Linux Trace Kernel
 help / color / mirror / Atom feed
* [PATCHv2] uprobes: Use flexible array for xol_area bitmap
@ 2026-05-11 22:56 Rosen Penev
  2026-05-12  0:48 ` Masami Hiramatsu
  2026-05-12 11:29 ` Oleg Nesterov
  0 siblings, 2 replies; 5+ messages in thread
From: Rosen Penev @ 2026-05-11 22:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, James Clark, Masami Hiramatsu,
	Oleg Nesterov, open list:PERFORMANCE EVENTS SUBSYSTEM,
	open list:UPROBES

The XOL slot bitmap has the same lifetime as struct xol_area, but it
is currently allocated separately.  That adds another allocation
failure path and a matching cleanup branch without buying any extra
flexibility.

Store the bitmap as a flexible array member and allocate it together
with the xol_area using kzalloc_flex().  The bitmap remains
zero-initialized, while the allocation and error handling become
simpler.

Assisted-by: Codex:GPT-5.5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: add missing kfree
 kernel/events/uprobes.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 4084e926e284..eba71700667e 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -108,7 +108,6 @@ static LIST_HEAD(delayed_uprobe_list);
  */
 struct xol_area {
 	wait_queue_head_t		wq;		/* if all slots are busy */
-	unsigned long			*bitmap;	/* 0 = free slot */
 
 	struct page			*page;
 	/*
@@ -117,6 +116,7 @@ struct xol_area {
 	 * the vma go away, and we must handle that reasonably gracefully.
 	 */
 	unsigned long			vaddr;		/* Page(s) of instruction slots */
+	unsigned long			bitmap[];	/* 0 = free slot */
 };
 
 static void uprobe_warn(struct task_struct *t, const char *msg)
@@ -1755,18 +1755,13 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 	struct xol_area *area;
 	void *insns;
 
-	area = kzalloc_obj(*area);
+	area = kzalloc_flex(*area, bitmap, BITS_TO_LONGS(UINSNS_PER_PAGE));
 	if (unlikely(!area))
 		goto out;
 
-	area->bitmap = kcalloc(BITS_TO_LONGS(UINSNS_PER_PAGE), sizeof(long),
-			       GFP_KERNEL);
-	if (!area->bitmap)
-		goto free_area;
-
 	area->page = alloc_page(GFP_HIGHUSER | __GFP_ZERO);
 	if (!area->page)
-		goto free_bitmap;
+		goto free_area;
 
 	area->vaddr = vaddr;
 	init_waitqueue_head(&area->wq);
@@ -1779,8 +1774,6 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 		return area;
 
 	__free_page(area->page);
- free_bitmap:
-	kfree(area->bitmap);
  free_area:
 	kfree(area);
  out:
@@ -1831,7 +1824,6 @@ void uprobe_clear_state(struct mm_struct *mm)
 		return;
 
 	put_page(area->page);
-	kfree(area->bitmap);
 	kfree(area);
 }
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCHv2] uprobes: Use flexible array for xol_area bitmap
  2026-05-11 22:56 [PATCHv2] uprobes: Use flexible array for xol_area bitmap Rosen Penev
@ 2026-05-12  0:48 ` Masami Hiramatsu
  2026-05-12 11:29 ` Oleg Nesterov
  1 sibling, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2026-05-12  0:48 UTC (permalink / raw)
  To: Rosen Penev
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	James Clark, Masami Hiramatsu, Oleg Nesterov,
	open list:PERFORMANCE EVENTS SUBSYSTEM, open list:UPROBES

On Mon, 11 May 2026 15:56:48 -0700
Rosen Penev <rosenp@gmail.com> wrote:

> The XOL slot bitmap has the same lifetime as struct xol_area, but it
> is currently allocated separately.  That adds another allocation
> failure path and a matching cleanup branch without buying any extra
> flexibility.
> 
> Store the bitmap as a flexible array member and allocate it together
> with the xol_area using kzalloc_flex().  The bitmap remains
> zero-initialized, while the allocation and error handling become
> simpler.
> 
> Assisted-by: Codex:GPT-5.5
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

OK, this looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks,

> ---
>  v2: add missing kfree
>  kernel/events/uprobes.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 4084e926e284..eba71700667e 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -108,7 +108,6 @@ static LIST_HEAD(delayed_uprobe_list);
>   */
>  struct xol_area {
>  	wait_queue_head_t		wq;		/* if all slots are busy */
> -	unsigned long			*bitmap;	/* 0 = free slot */
>  
>  	struct page			*page;
>  	/*
> @@ -117,6 +116,7 @@ struct xol_area {
>  	 * the vma go away, and we must handle that reasonably gracefully.
>  	 */
>  	unsigned long			vaddr;		/* Page(s) of instruction slots */
> +	unsigned long			bitmap[];	/* 0 = free slot */
>  };
>  
>  static void uprobe_warn(struct task_struct *t, const char *msg)
> @@ -1755,18 +1755,13 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
>  	struct xol_area *area;
>  	void *insns;
>  
> -	area = kzalloc_obj(*area);
> +	area = kzalloc_flex(*area, bitmap, BITS_TO_LONGS(UINSNS_PER_PAGE));
>  	if (unlikely(!area))
>  		goto out;
>  
> -	area->bitmap = kcalloc(BITS_TO_LONGS(UINSNS_PER_PAGE), sizeof(long),
> -			       GFP_KERNEL);
> -	if (!area->bitmap)
> -		goto free_area;
> -
>  	area->page = alloc_page(GFP_HIGHUSER | __GFP_ZERO);
>  	if (!area->page)
> -		goto free_bitmap;
> +		goto free_area;
>  
>  	area->vaddr = vaddr;
>  	init_waitqueue_head(&area->wq);
> @@ -1779,8 +1774,6 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
>  		return area;
>  
>  	__free_page(area->page);
> - free_bitmap:
> -	kfree(area->bitmap);
>   free_area:
>  	kfree(area);
>   out:
> @@ -1831,7 +1824,6 @@ void uprobe_clear_state(struct mm_struct *mm)
>  		return;
>  
>  	put_page(area->page);
> -	kfree(area->bitmap);
>  	kfree(area);
>  }
>  
> -- 
> 2.54.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCHv2] uprobes: Use flexible array for xol_area bitmap
  2026-05-11 22:56 [PATCHv2] uprobes: Use flexible array for xol_area bitmap Rosen Penev
  2026-05-12  0:48 ` Masami Hiramatsu
@ 2026-05-12 11:29 ` Oleg Nesterov
  2026-05-12 14:48   ` Masami Hiramatsu
  1 sibling, 1 reply; 5+ messages in thread
From: Oleg Nesterov @ 2026-05-12 11:29 UTC (permalink / raw)
  To: Rosen Penev
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	James Clark, Masami Hiramatsu,
	open list:PERFORMANCE EVENTS SUBSYSTEM, open list:UPROBES

On 05/11, Rosen Penev wrote:
>
>  struct xol_area {
>  	wait_queue_head_t		wq;		/* if all slots are busy */
> -	unsigned long			*bitmap;	/* 0 = free slot */
>
>  	struct page			*page;
>  	/*
> @@ -117,6 +116,7 @@ struct xol_area {
>  	 * the vma go away, and we must handle that reasonably gracefully.
>  	 */
>  	unsigned long			vaddr;		/* Page(s) of instruction slots */
> +	unsigned long			bitmap[];	/* 0 = free slot */
>  };
>
>  static void uprobe_warn(struct task_struct *t, const char *msg)
> @@ -1755,18 +1755,13 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
>  	struct xol_area *area;
>  	void *insns;
>
> -	area = kzalloc_obj(*area);
> +	area = kzalloc_flex(*area, bitmap, BITS_TO_LONGS(UINSNS_PER_PAGE));

The downside is that kmalloc will use kmem_cache with ->object_size = PAGE_SIZE * 2,
almost half of the allocated memory won't be used...

But technically the patch looks correct so I won't argue.

Oleg.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCHv2] uprobes: Use flexible array for xol_area bitmap
  2026-05-12 11:29 ` Oleg Nesterov
@ 2026-05-12 14:48   ` Masami Hiramatsu
  2026-05-12 16:17     ` Oleg Nesterov
  0 siblings, 1 reply; 5+ messages in thread
From: Masami Hiramatsu @ 2026-05-12 14:48 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Rosen Penev, linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	James Clark, Masami Hiramatsu,
	open list:PERFORMANCE EVENTS SUBSYSTEM, open list:UPROBES

On Tue, 12 May 2026 13:29:52 +0200
Oleg Nesterov <oleg@redhat.com> wrote:

> On 05/11, Rosen Penev wrote:
> >
> >  struct xol_area {
> >  	wait_queue_head_t		wq;		/* if all slots are busy */
> > -	unsigned long			*bitmap;	/* 0 = free slot */
> >
> >  	struct page			*page;
> >  	/*
> > @@ -117,6 +116,7 @@ struct xol_area {
> >  	 * the vma go away, and we must handle that reasonably gracefully.
> >  	 */
> >  	unsigned long			vaddr;		/* Page(s) of instruction slots */
> > +	unsigned long			bitmap[];	/* 0 = free slot */
> >  };
> >
> >  static void uprobe_warn(struct task_struct *t, const char *msg)
> > @@ -1755,18 +1755,13 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
> >  	struct xol_area *area;
> >  	void *insns;
> >
> > -	area = kzalloc_obj(*area);
> > +	area = kzalloc_flex(*area, bitmap, BITS_TO_LONGS(UINSNS_PER_PAGE));
> 
> The downside is that kmalloc will use kmem_cache with ->object_size = PAGE_SIZE * 2,
> almost half of the allocated memory won't be used...

Hmm, is the bitmap so big? 

#define UINSNS_PER_PAGE			(PAGE_SIZE/UPROBE_XOL_SLOT_BYTES)

And even on arm64, 

#define UPROBE_XOL_SLOT_BYTES	AARCH64_INSN_SIZE

So if PAGE_SIZE is 4k, UINSNS_PER_PAGE is 1k, its BITS_TO_LONGS will
be 1024/64 = 16. So 128 bytes. So the object is allocated from
object_size = 256 ?

Thank you,

> 
> But technically the patch looks correct so I won't argue.
> 
> Oleg.
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCHv2] uprobes: Use flexible array for xol_area bitmap
  2026-05-12 14:48   ` Masami Hiramatsu
@ 2026-05-12 16:17     ` Oleg Nesterov
  0 siblings, 0 replies; 5+ messages in thread
From: Oleg Nesterov @ 2026-05-12 16:17 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Rosen Penev, linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	James Clark, open list:PERFORMANCE EVENTS SUBSYSTEM,
	open list:UPROBES

On 05/12, Masami Hiramatsu wrote:
>
> On Tue, 12 May 2026 13:29:52 +0200
> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > >
> > > -	area = kzalloc_obj(*area);
> > > +	area = kzalloc_flex(*area, bitmap, BITS_TO_LONGS(UINSNS_PER_PAGE));
> >
> > The downside is that kmalloc will use kmem_cache with ->object_size = PAGE_SIZE * 2,
> > almost half of the allocated memory won't be used...
>
> Hmm, is the bitmap so big?
>
> #define UINSNS_PER_PAGE			(PAGE_SIZE/UPROBE_XOL_SLOT_BYTES)
>
> And even on arm64,
>
> #define UPROBE_XOL_SLOT_BYTES	AARCH64_INSN_SIZE
>
> So if PAGE_SIZE is 4k, UINSNS_PER_PAGE is 1k, its BITS_TO_LONGS will
> be 1024/64 = 16. So 128 bytes. So the object is allocated from
> object_size = 256 ?

Indeed you are right.

Sorry for the noise and thanks for correcting me! I can't even explain how can
I came to conclusion that object_size can be greater than PAGE_SIZE with this
change ;)

So I think the patch from Rosen is fine.

Thanks,

Oleg.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-12 16:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-11 22:56 [PATCHv2] uprobes: Use flexible array for xol_area bitmap Rosen Penev
2026-05-12  0:48 ` Masami Hiramatsu
2026-05-12 11:29 ` Oleg Nesterov
2026-05-12 14:48   ` Masami Hiramatsu
2026-05-12 16:17     ` Oleg Nesterov

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