On Wed 19-08-20 12:11:57, David Hildenbrand wrote: > Currently, it can happen that pages are allocated (and freed) via the buddy > before we finished basic memory onlining. > > For example, pages are exposed to the buddy and can be allocated before > we actually mark the sections online. Allocated pages could suddenly > fail pfn_to_online_page() checks. We had similar issues with pcp > handling, when pages are allocated+freed before we reach > zone_pcp_update() in online_pages() [1]. > > Instead, mark all pageblocks MIGRATE_ISOLATE, such that allocations are > impossible. Once done with the heavy lifting, use > undo_isolate_page_range() to move the pages to the MIGRATE_MOVABLE > freelist, marking them ready for allocation. Similar to offline_pages(), > we have to manually adjust zone->nr_isolate_pageblock. > > [1] > https://lkml.kernel.org/r/[email protected] > > Cc: Andrew Morton <[email protected]> > Cc: Michal Hocko <[email protected]> > Cc: Wei Yang <[email protected]> > Cc: Baoquan He <[email protected]> > Cc: Pankaj Gupta <[email protected]> > Cc: Oscar Salvador <[email protected]> > Cc: Charan Teja Reddy <[email protected]> > Signed-off-by: David Hildenbrand <[email protected]>
Acked-by: Michal Hocko <[email protected]> Yes this looks very sensible and we should have done that from the beginning. I just have one minor comment below > @@ -816,6 +816,14 @@ int __ref online_pages(unsigned long pfn, unsigned long > nr_pages, > if (ret) > goto failed_addition; > > + /* > + * Fixup the number of isolated pageblocks before marking the sections > + * onlining, such that undo_isolate_page_range() works correctly. > + */ > + spin_lock_irqsave(&zone->lock, flags); > + zone->nr_isolate_pageblock += nr_pages / pageblock_nr_pages; > + spin_unlock_irqrestore(&zone->lock, flags); > + I am not entirely happy about this. I am wondering whether it would make more sense to keep the counter in sync already in memmap_init_zone. Sure we add a branch to the boot time initialization - and it always fails there - but the code would be cleaner and we wouldn't have to do tricks like this in caller(s). -- Michal Hocko SUSE Labs
