[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <115b93eb-d6ae-9c48-f089-e381c9a66ff2@redhat.com>
Date: Thu, 25 Apr 2019 14:38:21 +0200
From: David Hildenbrand <david@...hat.com>
To: Pavel Tatashin <pasha.tatashin@...een.com>
Cc: Dan Williams <dan.j.williams@...el.com>,
James Morris <jmorris@...ei.org>,
Sasha Levin <sashal@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux MM <linux-mm@...ck.org>,
linux-nvdimm <linux-nvdimm@...ts.01.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Michal Hocko <mhocko@...e.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Keith Busch <keith.busch@...el.com>,
Vishal L Verma <vishal.l.verma@...el.com>,
Dave Jiang <dave.jiang@...el.com>,
Ross Zwisler <zwisler@...nel.org>,
Tom Lendacky <thomas.lendacky@....com>,
"Huang, Ying" <ying.huang@...el.com>,
Fengguang Wu <fengguang.wu@...el.com>,
Borislav Petkov <bp@...e.de>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Yaowei Bai <baiyaowei@...s.chinamobile.com>,
Takashi Iwai <tiwai@...e.de>,
Jérôme Glisse <jglisse@...hat.com>
Subject: Re: [v2 2/2] device-dax: "Hotremove" persistent memory that is used
like normal RAM
On 25.04.19 14:30, Pavel Tatashin wrote:
>>
>> Yes, also I think you can let go of the device_lock in
>> check_memblocks_offline_cb, lock_device_hotplug() should take care of
>> this (see Documentation/core-api/memory-hotplug.rst - "locking internals")
>>
> Hi David,
>
> Thank you for your comments. I went through memory-hotplug.rst, and I
> still think that device_lock() is needed here. In this particular case
> it can be replaced with something like READ_ONCE(), but for simplicity
> it is better to have device_lock()/device_unlock() as this is not a
> performance critical code.
>
> I do not see any lock ordering issues with this code, as we are
> holding lock_device_hotplug() first that prevents userland from
> adding/removing memory during this check.
Yes, lock ordering is not an issue, I rather think that the device
hotplug lock will guard us in all situations. E.g. remove_memory() also
does not use it when checking if all blocks are offline. But you can
leave it in if you think it is needed.
>
> https://soleen.com/source/xref/linux/arch/powerpc/platforms/powernv/memtrace.c?r=98fa15f3#248
>
> Here we have a similar code:
> lock_device_hotplug();
> online_mem_block();
> device_online()
> device_lock(dev);
>
> Pasha
>
--
Thanks,
David / dhildenb
Powered by blists - more mailing lists