On 18-11-05 13:19:45, Alexander Duyck wrote:
> }
> - first_init_pfn = max(zone->zone_start_pfn, first_init_pfn);
> +
> + /* If the zone is empty somebody else may have cleared out the zone */
> + if (!deferred_init_mem_pfn_range_in_zone(&i, zone, &spfn, &epfn,
> + first_init_pfn)) {
> + pgdat_resize_unlock(pgdat, &flags);
> + pgdat_init_report_one_done();
> + return 0;
It would make more sense to add goto to the end of this function and report
that in Node X had 0 pages initialized. Otherwise, it will be confusing
why some nodes are not initialized during boot. It is simply because
they do not have deferred pages left to be initialized.
> + }
>
> /*
> - * Initialize and free pages. We do it in two loops: first we initialize
> - * struct page, than free to buddy allocator, because while we are
> - * freeing pages we can access pages that are ahead (computing buddy
> - * page in __free_one_page()).
> + * Initialize and free pages in MAX_ORDER sized increments so
> + * that we can avoid introducing any issues with the buddy
> + * allocator.
> */
> - for_each_free_mem_pfn_range_in_zone(i, zone, &spfn, &epfn) {
> - spfn = max_t(unsigned long, first_init_pfn, spfn);
> - nr_pages += deferred_init_pages(zone, spfn, epfn);
> - }
> - for_each_free_mem_pfn_range_in_zone(i, zone, &spfn, &epfn) {
> - spfn = max_t(unsigned long, first_init_pfn, spfn);
> - deferred_free_pages(spfn, epfn);
> - }
> + while (spfn < epfn)
> + nr_pages += deferred_init_maxorder(&i, zone, &spfn, &epfn);
> +
> pgdat_resize_unlock(pgdat, &flags);
>
> /* Sanity check that the next zone really is unpopulated */
> @@ -1602,9 +1689,9 @@ deferred_grow_zone(struct zone *zone, unsigned int order)
> {
How about order = max(order, max_order)?
> unsigned long nr_pages_needed = ALIGN(1 << order, PAGES_PER_SECTION);
> - first_init_pfn = max(zone->zone_start_pfn, first_deferred_pfn);
> -
> - if (first_init_pfn >= pgdat_end_pfn(pgdat)) {
> + /* If the zone is empty somebody else may have cleared out the zone */
> + if (!deferred_init_mem_pfn_range_in_zone(&i, zone, &spfn, &epfn,
> + first_deferred_pfn)) {
> pgdat_resize_unlock(pgdat, &flags);
> - return false;
> + return true;
I am OK to change to true here, please also set
pgdat->first_deferred_pfn to ULONG_MAX to indicate that there are no
more deferred pages in this node.
Overall, I like this patch, makes things a lot easier, assuming the
above is addressed:
Reviewed-by: Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>
Thank you,
Pasha
