On Fri, Jun 02, 2023 at 11:13:54PM -0700, Dan Williams wrote:
> A CONFIG_DEBUG_KOBJECT_RELEASE test of removing a device-dax region
> provider (like modprobe -r dax_hmem) yields:
>
> kobject: 'mapping0' (ffff93eb460e8800): kobject_release, parent
> 0000000000000000 (delayed 2000)
> [..]
> DEBUG_LOCKS_WARN_ON(1)
> WARNING: CPU: 23 PID: 282 at kernel/locking/lockdep.c:232
> __lock_acquire+0x9fc/0x2260
> [..]
> RIP: 0010:__lock_acquire+0x9fc/0x2260
> [..]
> Call Trace:
> <TASK>
> [..]
> lock_acquire+0xd4/0x2c0
> ? ida_free+0x62/0x130
> _raw_spin_lock_irqsave+0x47/0x70
> ? ida_free+0x62/0x130
> ida_free+0x62/0x130
> dax_mapping_release+0x1f/0x30
> device_release+0x36/0x90
> kobject_delayed_cleanup+0x46/0x150
>
> Due to attempting ida_free() on an ida object that has already been
> freed. Devices typically only hold a reference on their parent while
> registered. If a child needs a parent object to complete its release it
> needs to hold a reference that it drops from its release callback.
> Arrange for a dax_mapping to pin its parent dev_dax instance until
> dax_mapping_release().
>
> Fixes: 0b07ce872a9e ("device-dax: introduce 'mapping' devices")
> Signed-off-by: Dan Williams <[email protected]>
> ---
Reviewed-by: Fan Ni <[email protected]>
> drivers/dax/bus.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 227800053309..aee695f86b44 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -635,10 +635,12 @@ EXPORT_SYMBOL_GPL(alloc_dax_region);
> static void dax_mapping_release(struct device *dev)
> {
> struct dax_mapping *mapping = to_dax_mapping(dev);
> - struct dev_dax *dev_dax = to_dev_dax(dev->parent);
> + struct device *parent = dev->parent;
> + struct dev_dax *dev_dax = to_dev_dax(parent);
>
> ida_free(&dev_dax->ida, mapping->id);
> kfree(mapping);
> + put_device(parent);
> }
>
> static void unregister_dax_mapping(void *data)
> @@ -778,6 +780,7 @@ static int devm_register_dax_mapping(struct dev_dax
> *dev_dax, int range_id)
> dev = &mapping->dev;
> device_initialize(dev);
> dev->parent = &dev_dax->dev;
> + get_device(dev->parent);
> dev->type = &dax_mapping_type;
> dev_set_name(dev, "mapping%d", mapping->id);
> rc = device_add(dev);
>
>
