| Message ID | 173989133862.230693.14094993331347437600.stgit@devnote2 (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 74e2498ccf7b303e7fdd881f58a849e884afb486 |
| Headers | show |
| Series | tracing: Make persistent ring buffer freeable | expand |
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); | ^~~~~
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,
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,
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 */