Re: [PATCH v5 8/9] dax/kmem: add sysfs interface for atomic whole-device hotplug

[Gregory Price <gourry@gourry.net>]

On Wed, Jun 24, 2026 at 03:11:21PM +0000, sashiko-bot@kernel.org 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 <gourry@gourry.net>
> 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/20260114235022.3437787-6-gourry@gourry.net/

   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