Messages in this thread | | | Subject | Re: [v2 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM | From | David Hildenbrand <> | Date | Thu, 25 Apr 2019 14:38:21 +0200 |
| |
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
| |