[PATCH v4 02/10] resource: add walk_system_ram_res_rev()
From: Julien Thierry <hidden>
Date: 2017-10-05 09:36:54
Also in:
kexec, lkml
Hi Takahiro, On 02/10/17 07:14, AKASHI Takahiro wrote:
quoted hunk ↗ jump to hunk
This function, being a variant of walk_system_ram_res() introduced in commit 8c86e70acead ("resource: provide new functions to walk through resources"), walks through a list of all the resources of System RAM in reversed order, i.e., from higher to lower. It will be used in kexec_file implementation on arm64. Signed-off-by: AKASHI Takahiro <redacted> Cc: Vivek Goyal <vgoyal@redhat.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> --- include/linux/ioport.h | 3 +++ kernel/resource.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+)diff --git a/include/linux/ioport.h b/include/linux/ioport.h index f5cf32e80041..62eb62b98118 100644 --- a/include/linux/ioport.h +++ b/include/linux/ioport.h@@ -273,6 +273,9 @@ extern int walk_system_ram_res(u64 start, u64 end, void *arg, int (*func)(u64, u64, void *)); extern int +walk_system_ram_res_rev(u64 start, u64 end, void *arg, + int (*func)(u64, u64, void *)); +extern int walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end, void *arg, int (*func)(u64, u64, void *));diff --git a/kernel/resource.c b/kernel/resource.c index 9b5f04404152..572f2f91ce9c 100644 --- a/kernel/resource.c +++ b/kernel/resource.c@@ -23,6 +23,8 @@ #include <linux/pfn.h> #include <linux/mm.h> #include <linux/resource_ext.h> +#include <linux/string.h> +#include <linux/vmalloc.h> #include <asm/io.h>@@ -469,6 +471,63 @@ int walk_system_ram_res(u64 start, u64 end, void *arg, return ret; } +int walk_system_ram_res_rev(u64 start, u64 end, void *arg, + int (*func)(u64, u64, void *)) +{ + struct resource res, *rams; + u64 orig_end;
nit: Why do you need orig_end? From what I can tell it is always equal to the "end" parameter of the function. If you think having orig_end makes it clearer to distinguish "end" from "res.end" could we declare it as: const u64 orig_end = end; Making it clear it is an alias?
+ int count, i; + int ret = -1; + + count = 16; /* initial */
nit: This doesn't represent the number of element we found but the size of the rams array. Would it be better named something like "rams_size"?
+
+ /* create a list */
+ rams = vmalloc(sizeof(struct resource) * count);
+ if (!rams)
+ return ret;
+
+ res.start = start;
+ res.end = end;
+ res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+ orig_end = res.end;
+ i = 0;
+ while ((res.start < res.end) &&
+ (!find_next_iomem_res(&res, IORES_DESC_NONE, true))) {
+ if (i >= count) {
+ /* re-alloc */
+ struct resource *rams_new;
+ int count_new;
+
+ count_new = count + 16;
+ rams_new = vmalloc(sizeof(struct resource) * count_new);
+ if (!rams_new)
+ goto out;Should we return -ENOMEM?
+ + memcpy(rams_new, rams, count);
We are likely to lose data here. -> memcpy(rams_new, rams, count * sizeof(struct resourse)); Also, if vremalloc doesn't exist maybe the realloc part could still be put in a separate function?
+ vfree(rams);
+ rams = rams_new;
+ count = count_new;
+ }
+
+ rams[i].start = res.start;
+ rams[i++].end = res.end;
+
+ res.start = res.end + 1;
+ res.end = orig_end;
+ }
+
+ /* go reverse */
+ for (i--; i >= 0; i--) {
+ ret = (*func)(rams[i].start, rams[i].end, arg);
+ if (ret)
+ break;
+ }
+
+out:
+ vfree(rams);
+ return ret;
+}
+
#if !defined(CONFIG_ARCH_HAS_WALK_MEMORY)
/*Cheers, -- Julien Thierry