[PATCH v4 0/2] tracing: Make persistent ring buffer freeable

[PATCH v4 0/2] tracing: Make persistent ring buffer freeable

[Masami Hiramatsu (Google)]

Hi,

Here is the 4th version of patches for making the persistent ring buffer
freeable. The previous version is here;

https://lore.kernel.org/all/173928521419.906035.17750338150436695675.stgit@devnote2/

In this version, I used free_reserved_area() instead of custom releasing
code. This seems working correctly. Thanks Mike!


Thank you,
---

Masami Hiramatsu (Google) (2):
      mm/memblock: Add reserved memory release function
      tracing: Freeable reserved ring buffer


 include/linux/mm.h   |    1 +
 kernel/trace/trace.c |   13 +++++++++-
 kernel/trace/trace.h |    1 +
 mm/memblock.c        |   66 +++++++++++++++++++++++++++++++++++++++++---------
 4 files changed, 68 insertions(+), 13 deletions(-)

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

[PATCH v4 1/2] mm/memblock: Add reserved memory release function

[Masami Hiramatsu (Google)]

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

Add reserve_mem_release_by_name() to release a reserved memory region
with a given name. This allows us to release reserved memory which is
defined by kernel cmdline, after boot.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
---
 Changes in v4:
  - Use free_reserved_area() according to Mike's comment.
 Changes in v2:
  - Rename reserved_mem_* to reserve_mem_*.
---
 include/linux/mm.h |    1 +
 mm/memblock.c      |   66 +++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7b1068ddcbb7..1ee9e7447485 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4123,6 +4123,7 @@ void vma_pgtable_walk_begin(struct vm_area_struct *vma);
 void vma_pgtable_walk_end(struct vm_area_struct *vma);
 
 int reserve_mem_find_by_name(const char *name, phys_addr_t *start, phys_addr_t *size);
+int reserve_mem_release_by_name(const char *name);
 
 #ifdef CONFIG_64BIT
 int do_mseal(unsigned long start, size_t len_in, unsigned long flags);
diff --git a/mm/memblock.c b/mm/memblock.c
index 95af35fd1389..8cd95f60015d 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -16,6 +16,7 @@
 #include <linux/kmemleak.h>
 #include <linux/seq_file.h>
 #include <linux/memblock.h>
+#include <linux/mutex.h>
 
 #include <asm/sections.h>
 #include <linux/io.h>
@@ -2283,6 +2284,7 @@ struct reserve_mem_table {
 };
 static struct reserve_mem_table reserved_mem_table[RESERVE_MEM_MAX_ENTRIES];
 static int reserved_mem_count;
+static DEFINE_MUTEX(reserve_mem_lock);
 
 /* Add wildcard region with a lookup name */
 static void __init reserved_mem_add(phys_addr_t start, phys_addr_t size,
@@ -2296,6 +2298,21 @@ static void __init reserved_mem_add(phys_addr_t start, phys_addr_t size,
 	strscpy(map->name, name);
 }
 
+static struct reserve_mem_table *reserve_mem_find_by_name_nolock(const char *name)
+{
+	struct reserve_mem_table *map;
+	int i;
+
+	for (i = 0; i < reserved_mem_count; i++) {
+		map = &reserved_mem_table[i];
+		if (!map->size)
+			continue;
+		if (strcmp(name, map->name) == 0)
+			return map;
+	}
+	return NULL;
+}
+
 /**
  * reserve_mem_find_by_name - Find reserved memory region with a given name
  * @name: The name that is attached to a reserved memory region
@@ -2309,22 +2326,47 @@ static void __init reserved_mem_add(phys_addr_t start, phys_addr_t size,
 int reserve_mem_find_by_name(const char *name, phys_addr_t *start, phys_addr_t *size)
 {
 	struct reserve_mem_table *map;
-	int i;
 
-	for (i = 0; i < reserved_mem_count; i++) {
-		map = &reserved_mem_table[i];
-		if (!map->size)
-			continue;
-		if (strcmp(name, map->name) == 0) {
-			*start = map->start;
-			*size = map->size;
-			return 1;
-		}
-	}
-	return 0;
+	guard(mutex)(&reserve_mem_lock);
+	map = reserve_mem_find_by_name_nolock(name);
+	if (!map)
+		return 0;
+
+	*start = map->start;
+	*size = map->size;
+	return 1;
 }
 EXPORT_SYMBOL_GPL(reserve_mem_find_by_name);
 
+/**
+ * reserve_mem_release_by_name - Release reserved memory region with a given name
+ * @name: The name that is attatched to a reserved memory region
+ *
+ * Forcibly release the pages in the reserved memory region so that those memory
+ * can be used as free memory. After released the reserved region size becomes 0.
+ *
+ * Returns: 1 if released or 0 if not found.
+ */
+int reserve_mem_release_by_name(const char *name)
+{
+	char buf[RESERVE_MEM_NAME_SIZE + 12];
+	struct reserve_mem_table *map;
+	void *start, *end;
+
+	guard(mutex)(&reserve_mem_lock);
+	map = reserve_mem_find_by_name_nolock(name);
+	if (!map)
+		return 0;
+
+	start = phys_to_virt(map->start);
+	end = start + map->size - 1;
+	snprintf(buf, sizeof(buf), "reserve_mem:%s", name);
+	free_reserved_area(start, end, 0, buf);
+	map->size = 0;
+
+	return 1;
+}
+
 /*
  * Parse reserve_mem=nn:align:name
  */

Re: [PATCH v4 1/2] mm/memblock: Add reserved memory release function

[Wei Yang]

On Wed, Feb 19, 2025 at 12:08:58AM +0900, Masami Hiramatsu (Google) wrote:
>From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
>Add reserve_mem_release_by_name() to release a reserved memory region
>with a given name. This allows us to release reserved memory which is
>defined by kernel cmdline, after boot.
>
>Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>Cc: Andrew Morton <akpm@linux-foundation.org>
>Cc: linux-mm@kvack.org
>---
> Changes in v4:
>  - Use free_reserved_area() according to Mike's comment.
> Changes in v2:
>  - Rename reserved_mem_* to reserve_mem_*.
>---
> include/linux/mm.h |    1 +
> mm/memblock.c      |   66 +++++++++++++++++++++++++++++++++++++++++++---------
> 2 files changed, 55 insertions(+), 12 deletions(-)
>
>diff --git a/include/linux/mm.h b/include/linux/mm.h
>index 7b1068ddcbb7..1ee9e7447485 100644
>--- a/include/linux/mm.h
>+++ b/include/linux/mm.h
>@@ -4123,6 +4123,7 @@ void vma_pgtable_walk_begin(struct vm_area_struct *vma);
> void vma_pgtable_walk_end(struct vm_area_struct *vma);
> 
> int reserve_mem_find_by_name(const char *name, phys_addr_t *start, phys_addr_t *size);
>+int reserve_mem_release_by_name(const char *name);
> 
> #ifdef CONFIG_64BIT
> int do_mseal(unsigned long start, size_t len_in, unsigned long flags);
>diff --git a/mm/memblock.c b/mm/memblock.c
>index 95af35fd1389..8cd95f60015d 100644
>--- a/mm/memblock.c
>+++ b/mm/memblock.c
>@@ -16,6 +16,7 @@
> #include <linux/kmemleak.h>
> #include <linux/seq_file.h>
> #include <linux/memblock.h>
>+#include <linux/mutex.h>
> 
> #include <asm/sections.h>
> #include <linux/io.h>
>@@ -2283,6 +2284,7 @@ struct reserve_mem_table {
> };
> static struct reserve_mem_table reserved_mem_table[RESERVE_MEM_MAX_ENTRIES];
> static int reserved_mem_count;
>+static DEFINE_MUTEX(reserve_mem_lock);
> 

This looks break the memblock tests in tools/testing/memblock.

memblock.c:2289:8: warning: type defaults to ‘int’ in declaration of ‘DEFINE_MUTEX’ [-Wimplicit-int]
 2289 | static DEFINE_MUTEX(reserve_mem_lock);
      |        ^~~~~~~~~~~~
memblock.c:2289:1: warning: parameter names (without types) in function declaration
 2289 | static DEFINE_MUTEX(reserve_mem_lock);
      | ^~~~~~
memblock.c: In function ‘reserve_mem_find_by_name’:
memblock.c:2332:9: warning: implicit declaration of function ‘guard’ [-Wimplicit-function-declaration]
 2332 |         guard(mutex)(&reserve_mem_lock);
      |         ^~~~~

Re: [PATCH v4 1/2] mm/memblock: Add reserved memory release function

[Masami Hiramatsu]

On Sat, 5 Apr 2025 02:30:18 +0000
Wei Yang <richard.weiyang@gmail.com> wrote:

> On Wed, Feb 19, 2025 at 12:08:58AM +0900, Masami Hiramatsu (Google) wrote:
> >From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >
> >Add reserve_mem_release_by_name() to release a reserved memory region
> >with a given name. This allows us to release reserved memory which is
> >defined by kernel cmdline, after boot.
> >
> >Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> >Cc: Andrew Morton <akpm@linux-foundation.org>
> >Cc: linux-mm@kvack.org
> >---
> > Changes in v4:
> >  - Use free_reserved_area() according to Mike's comment.
> > Changes in v2:
> >  - Rename reserved_mem_* to reserve_mem_*.
> >---
> > include/linux/mm.h |    1 +
> > mm/memblock.c      |   66 +++++++++++++++++++++++++++++++++++++++++++---------
> > 2 files changed, 55 insertions(+), 12 deletions(-)
> >
> >diff --git a/include/linux/mm.h b/include/linux/mm.h
> >index 7b1068ddcbb7..1ee9e7447485 100644
> >--- a/include/linux/mm.h
> >+++ b/include/linux/mm.h
> >@@ -4123,6 +4123,7 @@ void vma_pgtable_walk_begin(struct vm_area_struct *vma);
> > void vma_pgtable_walk_end(struct vm_area_struct *vma);
> > 
> > int reserve_mem_find_by_name(const char *name, phys_addr_t *start, phys_addr_t *size);
> >+int reserve_mem_release_by_name(const char *name);
> > 
> > #ifdef CONFIG_64BIT
> > int do_mseal(unsigned long start, size_t len_in, unsigned long flags);
> >diff --git a/mm/memblock.c b/mm/memblock.c
> >index 95af35fd1389..8cd95f60015d 100644
> >--- a/mm/memblock.c
> >+++ b/mm/memblock.c
> >@@ -16,6 +16,7 @@
> > #include <linux/kmemleak.h>
> > #include <linux/seq_file.h>
> > #include <linux/memblock.h>
> >+#include <linux/mutex.h>
> > 
> > #include <asm/sections.h>
> > #include <linux/io.h>
> >@@ -2283,6 +2284,7 @@ struct reserve_mem_table {
> > };
> > static struct reserve_mem_table reserved_mem_table[RESERVE_MEM_MAX_ENTRIES];
> > static int reserved_mem_count;
> >+static DEFINE_MUTEX(reserve_mem_lock);
> > 
> 
> This looks break the memblock tests in tools/testing/memblock.
> 
> memblock.c:2289:8: warning: type defaults to ‘int’ in declaration of ‘DEFINE_MUTEX’ [-Wimplicit-int]
>  2289 | static DEFINE_MUTEX(reserve_mem_lock);
>       |        ^~~~~~~~~~~~
> memblock.c:2289:1: warning: parameter names (without types) in function declaration
>  2289 | static DEFINE_MUTEX(reserve_mem_lock);
>       | ^~~~~~
> memblock.c: In function ‘reserve_mem_find_by_name’:
> memblock.c:2332:9: warning: implicit declaration of function ‘guard’ [-Wimplicit-function-declaration]
>  2332 |         guard(mutex)(&reserve_mem_lock);
>       |         ^~~~~

Hmm, this means the memblock test builds the kernel source code in user
space. I think we need to add linux/mutex.h under tools/testing/memblock.

But this is fragile by design. As I did for lib/bootconfig and
tools/bootconfig, you should use __KERNEL__ and makes it not depending on
the kernel header files because it does not expected to be used in user
space.
Even if I added mutex.h, it stopped with another reason.

test -L linux/memblock.h || ln -s ../../../../include/linux/memblock.h linux/memblock.h
test -L asm/asm.h || ln -s ../../../arch/x86/include/asm/asm.h asm/asm.h
test -L asm/cmpxchg.h || ln -s ../../../arch/x86/include/asm/cmpxchg.h asm/cmpxchg.h
cc -I. -I../../include -Wall -O2 -fsanitize=address -fsanitize=undefined -D CONFIG_PHYS_ADDR_T_64BIT   -c -o main.o main.c
test -L memblock.c || ln -s ../../../mm/memblock.c memblock.c
cc -I. -I../../include -Wall -O2 -fsanitize=address -fsanitize=undefined -D CONFIG_PHYS_ADDR_T_64BIT   -c -o memblock.o memblock.c
memblock.c: In function 'memblock_add_range.isra':
memblock.c:685:17: warning: 'end_rgn' may be used uninitialized [-Wmaybe-uninitialized]
  685 |                 memblock_merge_regions(type, start_rgn, end_rgn);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
memblock.c:591:42: note: 'end_rgn' was declared here
  591 |         int idx, nr_new, start_rgn = -1, end_rgn;
      |                                          ^~~~~~~
cc -I. -I../../include -Wall -O2 -fsanitize=address -fsanitize=undefined -D CONFIG_PHYS_ADDR_T_64BIT   -c -o lib/slab.o lib/slab.c
cc -I. -I../../include -Wall -O2 -fsanitize=address -fsanitize=undefined -D CONFIG_PHYS_ADDR_T_64BIT   -c -o mmzone.o mmzone.c
cc -I. -I../../include -Wall -O2 -fsanitize=address -fsanitize=undefined -D CONFIG_PHYS_ADDR_T_64BIT   -c -o slab.o ../../lib/slab.c
../../lib/slab.c:6:10: fatal error: urcu/uatomic.h: No such file or directory
    6 | #include <urcu/uatomic.h>
      |          ^~~~~~~~~~~~~~~~
compilation terminated.
make: *** [<builtin>: slab.o] Error 1


Thank you,

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

Re: [PATCH v4 1/2] mm/memblock: Add reserved memory release function

[Masami Hiramatsu]

On Mon, 7 Apr 2025 09:33:51 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> But this is fragile by design. As I did for lib/bootconfig and
> tools/bootconfig, you should use __KERNEL__ and makes it not depending on
> the kernel header files because it does not expected to be used in user
> space.
> Even if I added mutex.h, it stopped with another reason.
> 
> test -L linux/memblock.h || ln -s ../../../../include/linux/memblock.h linux/memblock.h
> test -L asm/asm.h || ln -s ../../../arch/x86/include/asm/asm.h asm/asm.h
> test -L asm/cmpxchg.h || ln -s ../../../arch/x86/include/asm/cmpxchg.h asm/cmpxchg.h
> cc -I. -I../../include -Wall -O2 -fsanitize=address -fsanitize=undefined -D CONFIG_PHYS_ADDR_T_64BIT   -c -o main.o main.c
> test -L memblock.c || ln -s ../../../mm/memblock.c memblock.c
> cc -I. -I../../include -Wall -O2 -fsanitize=address -fsanitize=undefined -D CONFIG_PHYS_ADDR_T_64BIT   -c -o memblock.o memblock.c
> memblock.c: In function 'memblock_add_range.isra':
> memblock.c:685:17: warning: 'end_rgn' may be used uninitialized [-Wmaybe-uninitialized]
>   685 |                 memblock_merge_regions(type, start_rgn, end_rgn);
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> memblock.c:591:42: note: 'end_rgn' was declared here
>   591 |         int idx, nr_new, start_rgn = -1, end_rgn;
>       |                                          ^~~~~~~
> cc -I. -I../../include -Wall -O2 -fsanitize=address -fsanitize=undefined -D CONFIG_PHYS_ADDR_T_64BIT   -c -o lib/slab.o lib/slab.c
> cc -I. -I../../include -Wall -O2 -fsanitize=address -fsanitize=undefined -D CONFIG_PHYS_ADDR_T_64BIT   -c -o mmzone.o mmzone.c
> cc -I. -I../../include -Wall -O2 -fsanitize=address -fsanitize=undefined -D CONFIG_PHYS_ADDR_T_64BIT   -c -o slab.o ../../lib/slab.c
> ../../lib/slab.c:6:10: fatal error: urcu/uatomic.h: No such file or directory
>     6 | #include <urcu/uatomic.h>
>       |          ^~~~~~~~~~~~~~~~
> compilation terminated.
> make: *** [<builtin>: slab.o] Error 1

Ah, sorry. This is from liburcu. I installed the package and fixes the issue.
Let me send the patch.

Thank you,

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

[PATCH v4 2/2] tracing: Freeable reserved ring buffer

[Masami Hiramatsu (Google)]

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

Make the ring buffer on reserved memory to be freeable. This allows us
to free the trace instance on the reserved memory without changing
cmdline and rebooting. Even if we can not change the kernel cmdline
for security reason, we can release the reserved memory for the ring
buffer as free (available) memory.

For example, boot kernel with reserved memory;
"reserve_mem=20M:2M:trace trace_instance=boot_mapped^traceoff@trace"

~ # free
              total        used        free      shared  buff/cache   available
Mem:        1995548       50544     1927568       14964       17436     1911480
Swap:             0           0           0
~ # rmdir /sys/kernel/tracing/instances/boot_mapped/
[   23.704023] Freeing reserve_mem:trace memory: 20476K
~ # free
              total        used        free      shared  buff/cache   available
Mem:        2016024       41844     1956740       14968       17440     1940572
Swap:             0           0           0

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v4:
  - Update the result example in the description.
 Changes in v3:
  - Remove debug printk.
---
 kernel/trace/trace.c |   13 ++++++++++++-
 kernel/trace/trace.h |    1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 0e6d517e74e0..cf57c7ac2c55 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9284,6 +9284,9 @@ static void free_trace_buffers(struct trace_array *tr)
 #ifdef CONFIG_TRACER_MAX_TRACE
 	free_trace_buffer(&tr->max_buffer);
 #endif
+
+	if (tr->range_addr_start)
+		vunmap((void *)tr->range_addr_start);
 }
 
 static void init_trace_flags_index(struct trace_array *tr)
@@ -9445,6 +9448,7 @@ trace_array_create_systems(const char *name, const char *systems,
 	free_cpumask_var(tr->pipe_cpumask);
 	free_cpumask_var(tr->tracing_cpumask);
 	kfree_const(tr->system_names);
+	kfree(tr->range_name);
 	kfree(tr->name);
 	kfree(tr);
 
@@ -9571,6 +9575,11 @@ static int __remove_instance(struct trace_array *tr)
 	free_trace_buffers(tr);
 	clear_tracing_err_log(tr);
 
+	if (tr->range_name) {
+		reserve_mem_release_by_name(tr->range_name);
+		kfree(tr->range_name);
+	}
+
 	for (i = 0; i < tr->nr_topts; i++) {
 		kfree(tr->topts[i].topts);
 	}
@@ -10368,6 +10377,7 @@ __init static void enable_instances(void)
 		bool traceoff = false;
 		char *flag_delim;
 		char *addr_delim;
+		char *rname __free(kfree) = NULL;
 
 		tok = strsep(&curr_str, ",");
 
@@ -10424,6 +10434,7 @@ __init static void enable_instances(void)
 				pr_warn("Failed to map boot instance %s to %s\n", name, tok);
 				continue;
 			}
+			rname = kstrdup(tok, GFP_KERNEL);
 		}
 
 		if (start) {
@@ -10460,7 +10471,7 @@ __init static void enable_instances(void)
 		 */
 		if (start) {
 			tr->flags |= TRACE_ARRAY_FL_BOOT;
-			tr->ref++;
+			tr->range_name = no_free_ptr(rname);
 		}
 
 		while ((tok = strsep(&curr_str, ","))) {
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 9c21ba45b7af..93b6279d3da4 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -348,6 +348,7 @@ struct trace_array {
 	unsigned int		mapped;
 	unsigned long		range_addr_start;
 	unsigned long		range_addr_size;
+	char			*range_name;
 	long			text_delta;
 	long			data_delta;
 

Re: [PATCH v4 0/2] tracing: Make persistent ring buffer freeable

[Steven Rostedt]

On Wed, 19 Feb 2025 00:08:47 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> Hi,
> 
> Here is the 4th version of patches for making the persistent ring buffer
> freeable. The previous version is here;
> 
> https://lore.kernel.org/all/173928521419.906035.17750338150436695675.stgit@devnote2/
> 
> In this version, I used free_reserved_area() instead of custom releasing
> code. This seems working correctly. Thanks Mike!
> 

Thanks Masami,

I'll start testing this after the fixes I have get tested. I'm currently
testing the fixes for the sorttable linux-next fixes, after that I'm going
to test the histogram fix, and then I'll test these patches.

-- Steve