| Message ID | 20170724190757.11278-10-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 24, 2017 at 02:07:49PM -0500, Brijesh Singh wrote: > From: Tom Lendacky <thomas.lendacky@amd.com> > > The walk_iomem_res_desc(), walk_system_ram_res() and walk_system_ram_range() > functions each have much of the same code. Create a new function that > consolidates the common code from these functions in one place to reduce > the amount of duplicated code. > > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > kernel/resource.c | 53 ++++++++++++++++++++++++++--------------------------- > 1 file changed, 26 insertions(+), 27 deletions(-) > > diff --git a/kernel/resource.c b/kernel/resource.c > index 9b5f044..7b20b3e 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c > @@ -397,9 +397,30 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc, > res->start = p->start; > if (res->end > p->end) > res->end = p->end; > + res->desc = p->desc; > return 0; I must be going blind: where are we using that res->desc? > +static int __walk_iomem_res_desc(struct resource *res, unsigned long desc, > + bool first_level_children_only, Btw, that variable name is insanely long. The rest looks ok to me, thanks for the cleanup!
On 7/28/2017 10:23 AM, Borislav Petkov wrote: > On Mon, Jul 24, 2017 at 02:07:49PM -0500, Brijesh Singh wrote: >> From: Tom Lendacky <thomas.lendacky@amd.com> >> >> The walk_iomem_res_desc(), walk_system_ram_res() and walk_system_ram_range() >> functions each have much of the same code. Create a new function that >> consolidates the common code from these functions in one place to reduce >> the amount of duplicated code. >> >> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> kernel/resource.c | 53 ++++++++++++++++++++++++++--------------------------- >> 1 file changed, 26 insertions(+), 27 deletions(-) >> >> diff --git a/kernel/resource.c b/kernel/resource.c >> index 9b5f044..7b20b3e 100644 >> --- a/kernel/resource.c >> +++ b/kernel/resource.c >> @@ -397,9 +397,30 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc, >> res->start = p->start; >> if (res->end > p->end) >> res->end = p->end; >> + res->desc = p->desc; >> return 0; > > I must be going blind: where are we using that res->desc? I think that was left-over from the initial consolidation work I was doing. I'll remove it. > >> +static int __walk_iomem_res_desc(struct resource *res, unsigned long desc, >> + bool first_level_children_only, > > Btw, that variable name is insanely long. I know, but I'm maintaining consistency with the name that was already present vs. changing it. > > The rest looks ok to me, thanks for the cleanup! Thanks, Tom >
On 8/17/2017 1:55 PM, Tom Lendacky wrote: > On 7/28/2017 10:23 AM, Borislav Petkov wrote: >> On Mon, Jul 24, 2017 at 02:07:49PM -0500, Brijesh Singh wrote: >>> From: Tom Lendacky <thomas.lendacky@amd.com> >>> >>> The walk_iomem_res_desc(), walk_system_ram_res() and >>> walk_system_ram_range() >>> functions each have much of the same code. Create a new function that >>> consolidates the common code from these functions in one place to reduce >>> the amount of duplicated code. >>> >>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> >>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >>> --- >>> kernel/resource.c | 53 >>> ++++++++++++++++++++++++++--------------------------- >>> 1 file changed, 26 insertions(+), 27 deletions(-) >>> >>> diff --git a/kernel/resource.c b/kernel/resource.c >>> index 9b5f044..7b20b3e 100644 >>> --- a/kernel/resource.c >>> +++ b/kernel/resource.c >>> @@ -397,9 +397,30 @@ static int find_next_iomem_res(struct resource >>> *res, unsigned long desc, >>> res->start = p->start; >>> if (res->end > p->end) >>> res->end = p->end; >>> + res->desc = p->desc; >>> return 0; >> >> I must be going blind: where are we using that res->desc? > > I think that was left-over from the initial consolidation work I was > doing. I'll remove it. I spoke too soon... I use it in a later patch as part of a callback. But instead of putting it here, I'll add it to the patch that actually needs it. Thanks, Tom > >> >>> +static int __walk_iomem_res_desc(struct resource *res, unsigned long >>> desc, >>> + bool first_level_children_only, >> >> Btw, that variable name is insanely long. > > I know, but I'm maintaining consistency with the name that was already > present vs. changing it. > >> >> The rest looks ok to me, thanks for the cleanup! > > Thanks, > Tom > >>
diff --git a/kernel/resource.c b/kernel/resource.c index 9b5f044..7b20b3e 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -397,9 +397,30 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc, res->start = p->start; if (res->end > p->end) res->end = p->end; + res->desc = p->desc; return 0; } +static int __walk_iomem_res_desc(struct resource *res, unsigned long desc, + bool first_level_children_only, + void *arg, int (*func)(u64, u64, void *)) +{ + u64 orig_end = res->end; + int ret = -1; + + while ((res->start < res->end) && + !find_next_iomem_res(res, desc, first_level_children_only)) { + ret = (*func)(res->start, res->end, arg); + if (ret) + break; + + res->start = res->end + 1; + res->end = orig_end; + } + + return ret; +} + /* * Walks through iomem resources and calls func() with matching resource * ranges. This walks through whole tree and not just first level children. @@ -418,26 +439,12 @@ int walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end, void *arg, int (*func)(u64, u64, void *)) { struct resource res; - u64 orig_end; - int ret = -1; res.start = start; res.end = end; res.flags = flags; - orig_end = res.end; - - while ((res.start < res.end) && - (!find_next_iomem_res(&res, desc, false))) { - - ret = (*func)(res.start, res.end, arg); - if (ret) - break; - - res.start = res.end + 1; - res.end = orig_end; - } - return ret; + return __walk_iomem_res_desc(&res, desc, false, arg, func); } /* @@ -451,22 +458,13 @@ int walk_system_ram_res(u64 start, u64 end, void *arg, int (*func)(u64, u64, void *)) { struct resource res; - u64 orig_end; - int ret = -1; res.start = start; res.end = end; res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - orig_end = res.end; - while ((res.start < res.end) && - (!find_next_iomem_res(&res, IORES_DESC_NONE, true))) { - ret = (*func)(res.start, res.end, arg); - if (ret) - break; - res.start = res.end + 1; - res.end = orig_end; - } - return ret; + + return __walk_iomem_res_desc(&res, IORES_DESC_NONE, true, + arg, func); } #if !defined(CONFIG_ARCH_HAS_WALK_MEMORY) @@ -508,6 +506,7 @@ static int __is_ram(unsigned long pfn, unsigned long nr_pages, void *arg) { return 1; } + /* * This generic page_is_ram() returns true if specified address is * registered as System RAM in iomem_resource list.