On 18-11-05 13:20:01, Alexander Duyck wrote:
> +static unsigned long __next_pfn_valid_range(unsigned long *i,
> + unsigned long end_pfn)
> {
> - if (!pfn_valid_within(pfn))
> - return false;
> - if (!(pfn & (pageblock_nr_pages - 1)) && !pfn_valid(pfn))
> - return false;
> - return true;
> + unsigned long pfn = *i;
> + unsigned long count;
> +
> + while (pfn < end_pfn) {
> + unsigned long t = ALIGN(pfn + 1, pageblock_nr_pages);
> + unsigned long pageblock_pfn = min(t, end_pfn);
> +
> +#ifndef CONFIG_HOLES_IN_ZONE
> + count = pageblock_pfn - pfn;
> + pfn = pageblock_pfn;
> + if (!pfn_valid(pfn))
> + continue;
> +#else
> + for (count = 0; pfn < pageblock_pfn; pfn++) {
> + if (pfn_valid_within(pfn)) {
> + count++;
> + continue;
> + }
> +
> + if (count)
> + break;
> + }
> +
> + if (!count)
> + continue;
> +#endif
> + *i = pfn;
> + return count;
> + }
> +
> + return 0;
> }
>
> +#define for_each_deferred_pfn_valid_range(i, start_pfn, end_pfn, pfn, count) \
> + for (i = (start_pfn), \
> + count = __next_pfn_valid_range(&i, (end_pfn)); \
> + count && ({ pfn = i - count; 1; }); \
> + count = __next_pfn_valid_range(&i, (end_pfn)))
Can this be improved somehow? It took me a while to understand this
piece of code. i is actually end of block, and not an index by PFN, ({pfn = i - count; 1;}) is
simply hard to parse. Why can't we make __next_pfn_valid_range() to
return both end and a start of a block?
The rest is good:
Reviewed-by: Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>
Thank you,
Pasha
