Messages in this thread | | | Date | Thu, 28 Aug 2008 18:02:48 -0700 | From | "Yinghai Lu" <> | Subject | Re: [PATCH] x86: split e820 reserved entries record to late v3 |
| |
On Thu, Aug 28, 2008 at 5:53 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > On Thu, 28 Aug 2008, Yinghai Lu wrote: >> /* >> * Mark e820 reserved areas as busy for the resource manager. >> */ >> +struct resource __initdata *e820_res; > > Btw, make this one "static" - I don't think anybody outside of this file > should ever use it anyway.
yes
> > And this: > >> + if (e820.map[i].type != E820_RESERVED || res->start < (1ULL<<20)) >> + insert_resource(&iomem_resource, res); > > is probably worthy of a comment.
how about /* * don't register the region that could be conflicted with pci device BAR resource * and insert them later. */
> > And finally: > >> +void __init e820_reserve_resources_late(void) >> +{ >> + int i; >> + struct resource *res; >> + >> + res = e820_res; >> + for (i = 0; i < e820.nr_map; i++) { >> + if (e820.map[i].type == E820_RESERVED && res->start >= (1ULL<<20)) >> + insert_resource(&iomem_resource, res); > > instead of duplicating the conditional and always having to worry about it > matching, how about just making it > > if (!res->parent) > insert_resource(&iomem_resource, res); > > which basically says that "if it's not already inserted, try to re-insert > it now". there could be one is all zero, when 32bit/!64_RESOURCE. so could change it to if (!res->parent && res->end) insert_resource(&iomem_resource, res);
YH
| |