On Tue 27-11-18 03:12:00, Wei Yang wrote: > On Mon, Nov 26, 2018 at 09:16:08AM +0100, Michal Hocko wrote: > >On Mon 26-11-18 10:28:40, Wei Yang wrote: > >[...] > >> But I get some difficulty to understand this TODO. You want to get rid of > >> these lock? While these locks seem necessary to protect those data of > >> pgdat/zone. Would you mind sharing more on this statement? > > > >Why do we need this lock to be irqsave? Is there any caller that uses > >the lock from the IRQ context? > > Went through the code, we have totally 9 place acquire > pgdat_resize_lock: > > lib/show_mem.c: 1 show_mem() > mm/memory_hotplug.c: 4 online/offline_pages/__remove_zone() > mm/page_alloc.c: 2 defer_init > mm/sparse.c: 2 not necessary > > Two places I am not sure: > > * show_mem() would be called from __alloc_pages_slowpath() This shouldn't really need the lock. It is a mostly debugging aid rather than something that cannot tolarate racing with hotplug. What is the worst case that can happen? > * __remove_zone() is related to acpi_scan() on x86, may related to > other method on different arch This one really needs a lock qwith a larger scope anyway. -- Michal Hocko SUSE Labs
