Re: [RFC] memory-layout-free zones (for review) [3/3] fix for_each_page_in_zone

Dave Hansen
2006-02-23T18:12:20Z

On Thu, 2006-02-23 at 18:00 +0900, KAMEZAWA Hiroyuki wrote:
> +static inline struct page *first_page_in_zone(struct zone *zone)
> +{
> + struct pglist_data *pgdat;
> + unsigned long start_pfn;
> + unsigned long i = 0;
> +
> + if (!populated_zone(zone))
> + return NULL;
> +
> + pgdat = zone->zone_pgdat;
> + zone = pgdat->node_start_pfn;
> +
> + for (i = 0; i < pgdat->zone_spanned_pages; i++) {
> + if (pfn_valid(start_pfn + i) && page_zone(page) == zone)
> + break;
> + }
> + BUG_ON(i == pgdat->node_spanned_pages); /* zone is populated */
> + return pfn_to_page(start_pfn + i);
> +}

I know we don't use this function _too_ much , but it would probably be
nice to make it a little smarter than "i++". We can be pretty sure, at
least with SPARSEMEM that the granularity is larger than that. We can
probably leave it until it gets to be a real problem.

I was also trying to think if a binary search is appropriate here. I
guess it depends on whether we allow the zones to have overlapping pfn
ranges, which I _think_ is one of the goals from these patches. Any
thoughts?

Oh, and I noticed the "pgdat->zone_spanned_pages" bit. Did you compile
this? ;)

> +static inline struct page *next_page_in_zone(struct page *page,
> + struct zone *zone)
> +{
> + struct pglist_data *pgdat;
> + unsigned long start_pfn;
> + unsigned long i;
> +
> + if (!populated_zone(zone))
> + return NULL;
> + pgdat = zone->zone_pgdat;
> + start_pfn = pgdat->node_start_pfn;
> + i = page_to_pfn(page) - start_pfn;
> +
> + for (i = i + 1; i < pgdat->node_spanned_pages; i++) {
> + if (pfn_vlaid(start_pfn + i) && page_zone(page) == zone)
> + break;
> + }
> + if (i == pgdat->node_spanned_pages)
> + return NULL;
> + return pfn_to_page(start_pfn + i);
> +}

Same comment, BTW, about code sharing. Is it something we want to or
can do with these?

-- Dave