Re: [v2 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM

From: David Hildenbrand <david@redhat.com>
To: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: "Dan Williams" <dan.j.williams@intel.com>,
	"James Morris" <jmorris@namei.org>,
	"Sasha Levin" <sashal@kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Linux MM" <linux-mm@kvack.org>,
	linux-nvdimm <linux-nvdimm@lists.01.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Michal Hocko" <mhocko@suse.com>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"Keith Busch" <keith.busch@intel.com>,
	"Vishal L Verma" <vishal.l.verma@intel.com>,
	"Dave Jiang" <dave.jiang@intel.com>,
	"Ross Zwisler" <zwisler@kernel.org>,
	"Tom Lendacky" <thomas.lendacky@amd.com>,
	"Huang, Ying" <ying.huang@intel.com>,
	"Fengguang Wu" <fengguang.wu@intel.com>,
	"Borislav Petkov" <bp@suse.de>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Yaowei Bai" <baiyaowei@cmss.chinamobile.com>,
	"Takashi Iwai" <tiwai@suse.de>,
	"Jérôme Glisse" <jglisse@redhat.com>
Subject: Re: [v2 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM
Date: Thu, 25 Apr 2019 14:38:21 +0200	[thread overview]
Message-ID: <115b93eb-d6ae-9c48-f089-e381c9a66ff2@redhat.com> (raw)
In-Reply-To: <CA+CK2bBt0vHr9D+BuvM=GmjCMESu5iBiUTdvid_TaoE6j2daQg@mail.gmail.com>

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