Skip to content
  • David Hildenbrand's avatar
    mm/memory_hotplug: fix online/offline_pages called w.o. mem_hotplug_lock · 381eab4a
    David Hildenbrand authored
    There seem to be some problems as result of 30467e0b ("mm, hotplug:
    fix concurrent memory hot-add deadlock"), which tried to fix a possible
    lock inversion reported and discussed in [1] due to the two locks
    	a) device_lock()
    	b) mem_hotplug_lock
    
    While add_memory() first takes b), followed by a) during
    bus_probe_device(), onlining of memory from user space first took a),
    followed by b), exposing a possible deadlock.
    
    In [1], and it was decided to not make use of device_hotplug_lock, but
    rather to enforce a locking order.
    
    The problems I spotted related to this:
    
    1. Memory block device attributes: While .state first calls
       mem_hotplug_begin() and the calls device_online() - which takes
       device_lock() - .online does no longer call mem_hotplug_begin(), so
       effectively calls online_pages() without mem_hotplug_lock.
    
    2. device_online() should be called under device_hotplug_lock, however
       onlining memory during add_memory() does not take care of that.
    
    In addition, I think there is also something wrong about the locking in
    
    3. arch/powerpc/platforms/powernv/memtrace.c calls offline_pages()
       without locks. This was introduced after 30467e0b. And skimming over
       the code, I assume it could need some more care in regards to locking
       (e.g. device_online() called without device_hotplug_lock. This will
       be addressed in the following patches.
    
    Now that we hold the device_hotplug_lock when
    - adding memory (e.g. via add_memory()/add_memory_resource())
    - removing memory (e.g. via remove_memory())
    - device_online()/device_offline()
    
    We can move mem_hotplug_lock usage back into
    online_pages()/offline_pages().
    
    Why is mem_hotplug_lock still needed? Essentially to make
    get_online_mems()/put_online_mems() be very fast (relying on
    device_hotplug_lock would be very slow), and to serialize against
    addition of memory that does not create memory block devices (hmm).
    
    [1] http://driverdev.linuxdriverproject.org/pipermail/ driverdev-devel/
        2015-February/065324.html
    
    This patch is partly based on a patch by Vitaly Kuznetsov.
    
    Link: http://lkml.kernel.org/r/20180925091457.28651-4-david@redhat.com
    
    
    Signed-off-by: default avatarDavid Hildenbrand <david@redhat.com>
    Reviewed-by: default avatarPavel Tatashin <pavel.tatashin@microsoft.com>
    Reviewed-by: default avatarRashmica Gupta <rashmica.g@gmail.com>
    Reviewed-by: default avatarOscar Salvador <osalvador@suse.de>
    Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
    Cc: Paul Mackerras <paulus@samba.org>
    Cc: Michael Ellerman <mpe@ellerman.id.au>
    Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
    Cc: Len Brown <lenb@kernel.org>
    Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Cc: "K. Y. Srinivasan" <kys@microsoft.com>
    Cc: Haiyang Zhang <haiyangz@microsoft.com>
    Cc: Stephen Hemminger <sthemmin@microsoft.com>
    Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
    Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
    Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
    Cc: Juergen Gross <jgross@suse.com>
    Cc: Rashmica Gupta <rashmica.g@gmail.com>
    Cc: Michael Neuling <mikey@neuling.org>
    Cc: Balbir Singh <bsingharora@gmail.com>
    Cc: Kate Stewart <kstewart@linuxfoundation.org>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: Philippe Ombredanne <pombredanne@nexb.com>
    Cc: Pavel Tatashin <pavel.tatashin@microsoft.com>
    Cc: Vlastimil Babka <vbabka@suse.cz>
    Cc: Dan Williams <dan.j.williams@intel.com>
    Cc: Oscar Salvador <osalvador@suse.de>
    Cc: YASUAKI ISHIMATSU <yasu.isimatu@gmail.com>
    Cc: Mathieu Malaterre <malat@debian.org>
    Cc: John Allen <jallen@linux.vnet.ibm.com>
    Cc: Jonathan Corbet <corbet@lwn.net>
    Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
    Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
    Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    381eab4a