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
legitamately 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.

Fixes: c05ae9d85b47 ("dax/bus.c: replace driver-core lock usage by a local 
rwsem")
Reported-by: Dan Williams <dan.j.willi...@intel.com>
Reviewed-by: Dan Williams <dan.j.willi...@intel.com>
Signed-off-by: Vishal Verma <vishal.l.ve...@intel.com>
---
 drivers/dax/bus.c | 42 ++++++++----------------------------------
 1 file changed, 8 insertions(+), 34 deletions(-)

diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index 7924dd542a13..e2c7354ce328 100644
--- a/drivers/dax/bus.c
+++ b/drivers/dax/bus.c
@@ -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);

-- 
2.44.0


Reply via email to