kernel/resource: make walk_system_ram_res() and walk_mem_res() search the whole tree
Subsystems (3)
v2 cover kernel/resource: make walk_system_ram_res() and walk_mem_res() search the whole tree
Playing with kdump+virtio-mem I noticed that kexec_file_load() does not consider System RAM added via dax/kmem and virtio-mem when preparing the elf header for kdump. Looking into the details, the logic used in walk_system_ram_res() and walk_mem_res() seems to be outdated.
walk_system_ram_range() already does the right thing, let's change walk_system_ram_res() and walk_mem_res(), and clean up.
Loading a kdump kernel via "kexec -p -s" ... will result in the kdump kernel to also dump dax/kmem and virtio-mem added System RAM now.
Note: kexec-tools on x86-64 also have to be updated to consider this memory in the kexec_load() case when processing /proc/iomem.
v1 -> v2:
- Added fixes tags to patch #1 and #2
- Refined the patch descriptions
- Added acks/rbs
David Hildenbrand (3):
kernel/resource: make walk_system_ram_res() find all busy
IORESOURCE_SYSTEM_RAM resources
kernel/resource: make walk_mem_res() find all busy IORESOURCE_MEM
resources
kernel/resource: remove first_lvl / siblings_only logic
kernel/resource.c | 45 ++++++++++++--------------------------------- 1 file changed, 12 insertions(+), 33 deletions(-)
base-commit: 0d02ec6b3136c73c09e7859f0d0e4e2c4c07b49b
Signature · 1 line
2.29.2
PATCH v2 1/3 kernel/resource: make walk_system_ram_res() find all busy IORESOURCE_SYSTEM_RAM resources
It used to be true that we can have system RAM (IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY) only on the first level in the resource tree. However, this is no longer holds for driver-managed system RAM (i.e., added via dax/kmem and virtio-mem), which gets added on lower levels, for example, inside device containers.
We have two users of walk_system_ram_res(), which currently only consideres the first level:
a) kernel/kexec_file.c:kexec_walk_resources() -- We properly skip IORESOURCE_SYSRAM_DRIVER_MANAGED resources via locate_mem_hole_callback(), so even after this change, we won't be placing kexec images onto dax/kmem and virtio-mem added memory. No change.
b) arch/x86/kernel/crash.c:fill_up_crash_elf_data() -- we're currently not adding relevant ranges to the crash elf header, resulting in them not getting dumped via kdump.
This change fixes loading a crashkernel via kexec_file_load() and including dax/kmem and virtio-mem added System RAM in the crashdump on x86-64. Note that e.g,, arm64 relies on memblock data and, therefore, always considers all added System RAM already.
Let's find all IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY resources, making the function behave like walk_system_ram_range().
--- kernel/resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/resource.c +1 -1
@@ -457,7 +457,7 @@ int walk_system_ram_res(u64 start, u64 end, void *arg, {unsigned long flags =IORESOURCE_SYSTEM_RAM |IORESOURCE_BUSY ; -return __walk_iomem_res_desc (start, end, flags,IORES_DESC_NONE , true, +return __walk_iomem_res_desc (start, end, flags,IORES_DESC_NONE , false, arg, func); }
Signature · 1 line
2.29.2
PATCH v2 2/3 kernel/resource: make walk_mem_res() find all busy IORESOURCE_MEM resources
In reply to David HildenbrandIt used to be true that we can have system RAM (IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY) only on the first level in the resource tree. However, this is no longer holds for driver-managed system RAM (i.e., added via dax/kmem and virtio-mem), which gets added on lower levels, for example, inside device containers.
IORESOURCE_SYSTEM_RAM is defined as IORESOURCE_MEM | IORESOURCE_SYSRAM and just a special type of IORESOURCE_MEM.
The function walk_mem_res() only consideres the first level and is used in arch/x86/mm/ioremap.c:__ioremap_check_mem() only. We currently fail to identify System RAM added by dax/kmem and virtio-mem as "IORES_MAP_SYSTEM_RAM", for example, allowing for remapping of such "normal RAM" in __ioremap_caller().
Let's find all IORESOURCE_MEM | IORESOURCE_BUSY resources, making the function behave similar to walk_system_ram_res().
--- kernel/resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/resource.c +1 -1
@@ -470,7 +470,7 @@ int walk_mem_res(u64 start, u64 end, void *arg, {unsigned long flags =IORESOURCE_MEM |IORESOURCE_BUSY ; -return __walk_iomem_res_desc (start, end, flags,IORES_DESC_NONE , true, +return __walk_iomem_res_desc (start, end, flags,IORES_DESC_NONE , false, arg, func); }
Signature · 1 line
2.29.2
PATCH v2 3/3 kernel/resource: remove first_lvl / siblings_only logic
In reply to David HildenbrandAll functions that search for IORESOURCE_SYSTEM_RAM or IORESOURCE_MEM resources now properly consider the whole resource tree, not just the first level. Let's drop the unused first_lvl / siblings_only logic.
Remove documentation that indicates that some functions behave differently, all consider the full resource tree now.
--- kernel/resource.c | 45 ++++++++++++--------------------------------- 1 file changed, 12 insertions(+), 33 deletions(-)
kernel/resource.c +12 -33
@@ -64,12 +64,8 @@ static DEFINE_RWLOCK(resource_lock);static struct resource *bootmem_resource_free;static DEFINE_SPINLOCK (bootmem_resource_lock); -static struct resource *next_resource (struct resource *p,bool sibling_only) +static struct resource *next_resource (struct resource *p) { -/* Caller wants to traverse through siblings only */ -if (sibling_only) -return p->sibling; -if (p->child)return p->child;while (!p->sibling && p->parent)
@@ -81,7 +77,7 @@ static void *r_next(struct seq_file *m, void *v, loff_t *pos) {struct resource *p = v; (*pos)++; -return (void *)next_resource (p, false); +return (void *)next_resource (p); }#ifdef CONFIG_PROC_FS
@@ -330,14 +326,10 @@ EXPORT_SYMBOL(release_resource);* of the resource that's within [@start..@end]; if none is found, returns * -ENODEV. Returns -EINVAL for invalid parameters. * -* This function walks the whole tree and not just first level children -* unless @first_lvl is true. -* * @start: start address of the resource searched for * @end: end address of same resource * @flags: flags which the resource must have * @desc: descriptor the resource must have -* @first_lvl: walk only the first level children, if set * @res: return ptr, if resource found * * The caller must specify @start, @end, @flags, and @desc
@@ -345,9 +337,8 @@ EXPORT_SYMBOL(release_resource);*/ static int find_next_iomem_res (resource_size_t start,resource_size_t end,unsigned long flags,unsigned long desc, -bool first_lvl,struct resource *res) +struct resource *res) { -bool siblings_only = true;struct resource *p;if (!res)
@@ -358,7 +349,7 @@ static int find_next_iomem_res(resource_size_t start, resource_size_t end,read_lock (&resource_lock); -for (p = iomem_resource.child; p; p =next_resource (p, siblings_only)) { +for (p = iomem_resource.child; p; p =next_resource (p)) {/* If we passed the resource we are looking for, stop */ if (p->start > end) { p =NULL ;
@@ -369,13 +360,6 @@ static int find_next_iomem_res(resource_size_t start, resource_size_t end,if (p->end < start)continue ; -/* -* Now that we found a range that matches what we look for, -* check the flags and the descriptor. If we were not asked to -* use only the first level, start looking at children as well. -*/ - siblings_only = first_lvl; -if ((p->flags & flags) != flags)continue ;if ((desc !=IORES_DESC_NONE ) && (desc != p->desc))
@@ -402,14 +386,14 @@ static int find_next_iomem_res(resource_size_t start, resource_size_t end,static int __walk_iomem_res_desc (resource_size_t start,resource_size_t end,unsigned long flags,unsigned long desc, -bool first_lvl,void *arg, +void *arg,int (*func)(struct resource *,void *)) {struct resource res;int ret = -EINVAL ;while (start < end && - !find_next_iomem_res (start, end, flags, desc, first_lvl, &res)) { + !find_next_iomem_res (start, end, flags, desc, &res)) { ret = (*func)(&res, arg);if (ret)break ;
@@ -431,7 +415,6 @@ static int __walk_iomem_res_desc(resource_size_t start, resource_size_t end,* @arg: function argument for the callback @func * @func: callback function that is called for each qualifying resource area * -* This walks through whole tree and not just first level children. * All the memory ranges which overlap start,end and also match flags and * desc are valid candidates. *
@@ -441,7 +424,7 @@ static int __walk_iomem_res_desc(resource_size_t start, resource_size_t end,int walk_iomem_res_desc (unsigned long desc,unsigned long flags,u64 start,u64 end,void *arg,int (*func)(struct resource *,void *)) { -return __walk_iomem_res_desc (start, end, flags, desc, false, arg, func); +return __walk_iomem_res_desc (start, end, flags, desc, arg, func); }EXPORT_SYMBOL_GPL (walk_iomem_res_desc);
@@ -457,8 +440,8 @@ int walk_system_ram_res(u64 start, u64 end, void *arg, {unsigned long flags =IORESOURCE_SYSTEM_RAM |IORESOURCE_BUSY ; -return __walk_iomem_res_desc (start, end, flags,IORES_DESC_NONE , false, - arg, func); +return __walk_iomem_res_desc (start, end, flags,IORES_DESC_NONE , arg, + func); }/*
@@ -470,17 +453,14 @@ int walk_mem_res(u64 start, u64 end, void *arg, {unsigned long flags =IORESOURCE_MEM |IORESOURCE_BUSY ; -return __walk_iomem_res_desc (start, end, flags,IORES_DESC_NONE , false, - arg, func); +return __walk_iomem_res_desc (start, end, flags,IORES_DESC_NONE , arg, + func); }/* * This function calls the @func callback against all memory ranges of type * System RAM which are marked as IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY. * It is to be used only for System RAM. -* -* This will find System RAM ranges that are children of top-level resources -* in addition to top-level System RAM resources. */ int walk_system_ram_range (unsigned long start_pfn,unsigned long nr_pages,void *arg,int (*func)(unsigned long ,unsigned long ,void *))
@@ -495,8 +475,7 @@ int walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages, end = ((u64)(start_pfn + nr_pages) <<PAGE_SHIFT ) -1 ; flags =IORESOURCE_SYSTEM_RAM |IORESOURCE_BUSY ;while (start < end && - !find_next_iomem_res (start, end, flags,IORES_DESC_NONE , - false, &res)) { + !find_next_iomem_res (start, end, flags,IORES_DESC_NONE , &res)) { pfn =PFN_UP (res.start); end_pfn =PFN_DOWN (res.end +1 );if (end_pfn > pfn)
Signature · 1 line
2.29.2