lkml.org 
[lkml]   [2006]   [May]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Lhms-devel] [RFC][PATCH] hot add memory which is not aligned to section
From
Date
On Thu, 2006-04-27 at 22:37 +0900, KAMEZAWA Hiroyuki wrote:
> @@ -145,10 +148,26 @@
> if (!populated_zone(zone))
> need_zonelists_rebuild = 1;
>
> - for (i = 0; i < nr_pages; i++) {
> - struct page *page = pfn_to_page(pfn + i);
> - online_page(page);
> - onlined_pages++;
> + res.start = (u64)pfn << PAGE_SHIFT;
> + res.end = res.start + ((u64)nr_pages << PAGE_SHIFT) - 1;
> + section_end = res.end;
> +
> + while (find_next_system_ram(&res)) {
> + start_pfn = (unsigned long)(res.start >> PAGE_SHIFT);
> + nr_pages = (unsigned long)
> + ((res.end + 1 - res.start) >> PAGE_SHIFT);
> +
> + if (PageReserved(pfn_to_page(start_pfn))) {
> + /* this region's page is not populated now */
> + for (i = 0; i < nr_pages; i++) {
> + struct page *page = pfn_to_page(start_pfn + i);
> + online_page(page);
> + onlined_pages++;
> + }
> + }
> +
> + res.start = res.end + 1;
> + res.end = section_end;
> }
> zone->present_pages += onlined_pages;
> zone->zone_pgdat->node_present_pages += onlined_pages;

First of all, bravo for doing this in an architecture-independent way.
Very nice.

I'd really prefer to keep the 'struct resource' handling out of the
memory hotplug code. This took a nice, little, comprehensible for loop,
and made it quite a bit more complex. There is also a lot of casting
going on, which I don't really grasp in a first glance.

The 'struct resource' which gets passed into find_next_system_ram()
isn't a real resource. Why not just pass a normal start and end address
in there, and let _it_ do the work?

It looks like that whole loop is optimized for being able to online a
really sparse area without diving into the iomem tables very often.
This seems like a premature complicating optimization to me.

Why not do something like this:

for (i = 0; i < nr_pages; i++) {
struct page *page = pfn_to_page(pfn + i);

if (page_is_in_io_resource(page))
continue;

online_page(page);
onlined_pages++;
}

That way, you keep the memory_hotplug.c file nice and neat.

Also, remind me again why you can't just make the SECTION_SIZE match
your 64MB I/O hole sizes. I forget a lot/ :)

-- Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-05-01 16:59    [W:0.068 / U:0.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site