On Wed, Jun 24, 2026 at 03:11:21PM +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) > to consider: > - [High] The protection against deadlocks during driver unbind can be > completely bypassed if the underlying memory blocks are manually onlined via > the core memory sysfs, causing state desynchronization. > > commit fa89899811083ada324ea3db8204b1c135005af5 > Author: Gregory Price <[email protected]> > Subject: dax/kmem: add sysfs interface for atomic whole-device hotplug > > > @@ -348,20 +490,26 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) > > #ifdef CONFIG_MEMORY_HOTREMOVE > > static void dev_dax_kmem_remove(struct dev_dax *dev_dax) > > { > > [ ... ] > > > + if (dax_kmem_state_is_online(data->state)) { > > + dev_warn(dev, "Hotplug regions stuck online until reboot\n"); > > + any_hotremove_failed = true; > > + return; > > + } else if (data->state == MMOP_OFFLINE && > > + dax_kmem_do_hotremove(dev_dax, data)) { > > + dev_warn(dev, "Unplug failed, resources leaked until reboot\n"); > > return; > > } > > [Severity: High] > Can this deadlock during unbind if the memory state is desynchronized? > > If the device is configured with auto_online_blocks=offline, data->state is > initially MMOP_OFFLINE. If a user manually onlines the underlying memory > blocks via the generic /sys/devices/system/memory/memoryX/state sysfs > interface, data->state remains MMOP_OFFLINE. > > During unbind, dev_dax_kmem_remove() will evaluate > dax_kmem_state_is_online(data->state) as false and proceed to call > dax_kmem_do_hotremove() instead of safely aborting. This attempts to offline > the blocks while holding the device unbind lock, bypassing the intended > deadlock protection. >
Pulling this feedback onto the list so you know i'm not going to address both pieces here when it comes back in the next version. 1) dax/state and memoryN/state can become desynced. We can't do anything about this without a memory notifier that prevents this desync by disallowing memoryN/state toggling for those blocks. https://lore.kernel.org/linux-mm/[email protected]/ this unfortunately breaks existing userland tools. I was told to drop the notifier path and explore deprecation at a later time. 2) What I will address: if state=UNPLUGGED we can safely proceed, otherwise we'll simply fall back to the old remove_memory() behavior where deadlocks are still possible. I'll add comments that address this. ~Gregory
