dax/bus.c: fix locking for unregister_dax_dev / unregister_dax_mapping paths
authorVishal Verma <vishal.l.verma@intel.com>
Tue, 30 Apr 2024 17:44:24 +0000 (11:44 -0600)
committerAndrew Morton <akpm@linux-foundation.org>
Tue, 7 May 2024 17:37:00 +0000 (10:37 -0700)
Commit c05ae9d85b47 ("dax/bus.c: replace driver-core lock usage by a local
rwsem") aimed to undo device_lock() abuses for protecting changes to
dax-driver internal data-structures like the dax_region resource tree to
device-dax-instance range structures.  However, the device_lock() was
legitimately enforcing that devices to be deleted were not current
actively attached to any driver nor assigned any capacity from the region.

As a result of the device_lock restoration in delete_store(), the
conditional locking in unregister_dev_dax() and unregister_dax_mapping()
can be removed.

Link: https://lkml.kernel.org/r/20240430-vv-dax_abi_fixes-v3-2-e3dcd755774c@intel.com
Fixes: c05ae9d85b47 ("dax/bus.c: replace driver-core lock usage by a local rwsem")
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Reported-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Cc: Alison Schofield <alison.schofield@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
drivers/dax/bus.c

index 7924dd542a13986bc62f11c10f5f64050d17ee96..e2c7354ce3281b307c9ff36b9a2944b11f53f7ad 100644 (file)
@@ -465,26 +465,17 @@ static void free_dev_dax_ranges(struct dev_dax *dev_dax)
                trim_dev_dax_range(dev_dax);
 }
 
-static void __unregister_dev_dax(void *dev)
+static void unregister_dev_dax(void *dev)
 {
        struct dev_dax *dev_dax = to_dev_dax(dev);
 
        dev_dbg(dev, "%s\n", __func__);
 
+       down_write(&dax_region_rwsem);
        kill_dev_dax(dev_dax);
        device_del(dev);
        free_dev_dax_ranges(dev_dax);
        put_device(dev);
-}
-
-static void unregister_dev_dax(void *dev)
-{
-       if (rwsem_is_locked(&dax_region_rwsem))
-               return __unregister_dev_dax(dev);
-
-       if (WARN_ON_ONCE(down_write_killable(&dax_region_rwsem) != 0))
-               return;
-       __unregister_dev_dax(dev);
        up_write(&dax_region_rwsem);
 }
 
@@ -560,15 +551,10 @@ static ssize_t delete_store(struct device *dev, struct device_attribute *attr,
        if (!victim)
                return -ENXIO;
 
-       rc = down_write_killable(&dax_region_rwsem);
-       if (rc)
-               return rc;
-       rc = down_write_killable(&dax_dev_rwsem);
-       if (rc) {
-               up_write(&dax_region_rwsem);
-               return rc;
-       }
+       device_lock(dev);
+       device_lock(victim);
        dev_dax = to_dev_dax(victim);
+       down_write(&dax_dev_rwsem);
        if (victim->driver || dev_dax_size(dev_dax))
                rc = -EBUSY;
        else {
@@ -589,11 +575,12 @@ static ssize_t delete_store(struct device *dev, struct device_attribute *attr,
                        rc = -EBUSY;
        }
        up_write(&dax_dev_rwsem);
+       device_unlock(victim);
 
        /* won the race to invalidate the device, clean it up */
        if (do_del)
                devm_release_action(dev, unregister_dev_dax, victim);
-       up_write(&dax_region_rwsem);
+       device_unlock(dev);
        put_device(victim);
 
        return rc;
@@ -705,7 +692,7 @@ static void dax_mapping_release(struct device *dev)
        put_device(parent);
 }
 
-static void __unregister_dax_mapping(void *data)
+static void unregister_dax_mapping(void *data)
 {
        struct device *dev = data;
        struct dax_mapping *mapping = to_dax_mapping(dev);
@@ -713,25 +700,12 @@ static void __unregister_dax_mapping(void *data)
 
        dev_dbg(dev, "%s\n", __func__);
 
-       lockdep_assert_held_write(&dax_region_rwsem);
-
        dev_dax->ranges[mapping->range_id].mapping = NULL;
        mapping->range_id = -1;
 
        device_unregister(dev);
 }
 
-static void unregister_dax_mapping(void *data)
-{
-       if (rwsem_is_locked(&dax_region_rwsem))
-               return __unregister_dax_mapping(data);
-
-       if (WARN_ON_ONCE(down_write_killable(&dax_region_rwsem) != 0))
-               return;
-       __unregister_dax_mapping(data);
-       up_write(&dax_region_rwsem);
-}
-
 static struct dev_dax_range *get_dax_range(struct device *dev)
 {
        struct dax_mapping *mapping = to_dax_mapping(dev);