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

>>> +static int >>> +offline_memblock_cb(struct memory_block *mem, void *arg) >> >> Function name suggests that you are actually trying to offline memory >> here. Maybe check_memblocks_offline_cb(), just like we have in >> mm/memory_hotplug.c. Makes sense, I will rename to check_memblocks_offline_cb() >>> + lock_device_hotplug(); >>> + rc = walk_memory_range(start_pfn, end_pfn, dev, offline_memblock_cb); >>> + unlock_device_hotplug(); >>> + >>> + /* >>> + * If admin has not offlined memory beforehand, we cannot hotremove dax. >>> + * Unfortunately, because unbind will still succeed there is no way for >>> + * user to hotremove dax after this. >>> + */ >>> + if (rc) >>> + return rc; >> >> Can't it happen that there is a race between you checking if memory is >> offline and an admin onlining memory again? maybe pull the >> remove_memory() into the locked region, using __remove_memory() instead. > > I think the race is ok. The admin gets to keep the pieces of allowing > racing updates to the state and the kernel will keep the range active > until the next reboot. Thank you for noticing this. I will pull it into locking region. Because, __remove_memory() has this code: 1868 ret = walk_memory_range(PFN_DOWN(start), PFN_UP(start + size - 1), NULL, 1869 check_memblock_offlined_cb); 1870 if (ret) 1871 BUG();