On Thu 22-11-18 01:52:39, Wei Yang wrote: > On Wed, Nov 21, 2018 at 08:15:49AM +0100, Michal Hocko wrote: [...] > >diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > >index c6c42a7425e5..c75fca900044 100644 > >--- a/mm/memory_hotplug.c > >+++ b/mm/memory_hotplug.c > >@@ -743,13 +743,12 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn, > > int nid = pgdat->node_id; > > unsigned long flags; > > > >+ /* TODO Huh pgdat is irqsave while zone is not. It used to be like that before */ > >+ pgdat_resize_lock(pgdat, &flags); > > if (zone_is_empty(zone)) > > init_currently_empty_zone(zone, start_pfn, nr_pages); > > > > clear_zone_contiguous(zone); > >- > >- /* TODO Huh pgdat is irqsave while zone is not. It used to be like that before */ > >- pgdat_resize_lock(pgdat, &flags); > > zone_span_writelock(zone); > > Just want to make sure, Oscar suggests to move the code here to protect > this under zone_span_lock. Yes, both locks held is probably safer. Because there is both pgdat and zone state updated. Sorry to confuse you -- Michal Hocko SUSE Labs