From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by kanga.kvack.org (Postfix) with ESMTP id B06CC8E0001 for ; Mon, 17 Sep 2018 03:32:43 -0400 (EDT) Received: by mail-qk1-f198.google.com with SMTP id a70-v6so13703362qkb.16 for ; Mon, 17 Sep 2018 00:32:43 -0700 (PDT) Received: from mx1.redhat.com (mx1.redhat.com. [209.132.183.28]) by mx.google.com with ESMTPS id m18-v6si4653463qka.155.2018.09.17.00.32.42 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 17 Sep 2018 00:32:42 -0700 (PDT) Subject: Re: [PATCH RFCv2 3/6] mm/memory_hotplug: fix online/offline_pages called w.o. mem_hotplug_lock References: <20180821104418.12710-1-david@redhat.com> <20180821104418.12710-4-david@redhat.com> <70372ef5-e332-6c07-f08c-50f8808bde6d@gmail.com> From: David Hildenbrand Message-ID: <5f80ca56-9f34-4e6e-bc83-8f8b3c888163@redhat.com> Date: Mon, 17 Sep 2018 09:32:27 +0200 MIME-Version: 1.0 In-Reply-To: <70372ef5-e332-6c07-f08c-50f8808bde6d@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Rashmica , linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-acpi@vger.kernel.org, xen-devel@lists.xenproject.org, devel@linuxdriverproject.org, Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , "Rafael J. Wysocki" , Len Brown , Greg Kroah-Hartman , "K. Y. Srinivasan" , Haiyang Zhang , Stephen Hemminger , Martin Schwidefsky , Heiko Carstens , Boris Ostrovsky , Juergen Gross , Michael Neuling , Balbir Singh , Kate Stewart , Thomas Gleixner , Philippe Ombredanne , Andrew Morton , Michal Hocko , Pavel Tatashin , Vlastimil Babka , Dan Williams , Oscar Salvador , YASUAKI ISHIMATSU , Mathieu Malaterre Am 03.09.18 um 02:36 schrieb Rashmica: > Hi David, > > > On 21/08/18 20:44, David Hildenbrand wrote: > >> There seem to be some problems as result of 30467e0b3be ("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 b), >> followed by a), exposing a possible deadlock. > > Do you mean "onlining of memory from user space first took a), > followed by b)"? Very right, thanks. > >> 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 30467e0b3be. 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 - but I'll >> not touch that for now). > > Can you mention that you fixed this in later patches? Sure! > > > The series looks good to me. Feel free to add my reviewed-by: > > Reviewed-by: Rashmica Gupta > Thanks, r-b only for this patch or all of the series? -- Thanks, David / dhildenb