On Mon 26-11-18 23:17:40, Dave Hansen wrote: > On 11/26/18 10:25 PM, Michal Hocko wrote: > > [Cc Dave who has added the lock into this path. Maybe he remembers why] > > I don't remember specifically. But, the pattern of: > > allocate > lock > set > unlock > > is _usually_ so we don't have two "sets" racing with each other. In > this case, that would have been to ensure that two > sparse_init_one_section()'s didn't race and leak one of the two > allocated memmaps or worse. > > I think mem_hotplug_lock protects this case these days, though. I don't > think we had it in the early days and were just slumming it with the > pgdat locks. > > I really don't like the idea of removing the lock by just saying it > doesn't protect anything without doing some homework first, though. It > would actually be really nice to comment the entire call chain from the > mem_hotplug_lock acquisition to here. There is precious little > commenting in there and it could use some love. Agreed. It really seems like the lock is not needed anymore but a more trhougout analysis and explanation is definitely due. Thanks! -- Michal Hocko SUSE Labs
