dax/bus.c: replace driver-core lock usage by a local rwsem
Patch series "Add DAX ABI for memmap_on_memory", v7. This series adds sysfs ABI to control memmap_on_memory behavior for DAX devices. Patch 1 replaces incorrect device_lock() usage with a local rwsem - this was identified during review. Patch 2 is also a preparatory patch that replaces sprintf() for sysfs operations with sysfs_emit() Patch 3 adds the missing documentation for the sysfs ABI for DAX regions and Dax devices. Patch 4 exports mhp_supports_memmap_on_memory(). Patch 5 adds the new ABI for toggling memmap_on_memory semantics for dax devices. This patch (of 5): The dax driver incorrectly used driver-core device locks to protect internal dax region and dax device configuration structures. Replace the device lock usage with a local rwsem, one each for dax region configuration and dax device configuration. As a result of this conversion, no device_lock() usage remains in dax/bus.c. Link: https://lkml.kernel.org/r/20240124-vv-dax_abi-v7-0-20d16cb8d23d@intel.com Link: https://lkml.kernel.org/r/20240124-vv-dax_abi-v7-1-20d16cb8d23d@intel.com Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reviewed-by: Alison Schofield <alison.schofield@intel.com> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Dave Hansen <dave.hansen@linux.intel.com> Cc: Dave Jiang <dave.jiang@intel.com> Cc: David Hildenbrand <david@redhat.com> Cc: Huang Ying <ying.huang@intel.com> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> Cc: Li Zhijian <lizhijian@fujitsu.com> Cc: Matthew Wilcox (Oracle) <willy@infradead.org> Cc: Michal Hocko <mhocko@suse.com> Cc: Oscar Salvador <osalvador@suse.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
This commit is contained in:
Vishal Verma
Andrew Morton
parent
db128f5fde
commit
c05ae9d85b
@ -12,6 +12,18 @@
|
||||
|
||||
static DEFINE_MUTEX(dax_bus_lock);
|
||||
|
||||
/*
|
||||
* All changes to the dax region configuration occur with this lock held
|
||||
* for write.
|
||||
*/
|
||||
DECLARE_RWSEM(dax_region_rwsem);
|
||||
|
||||
/*
|
||||
* All changes to the dax device configuration occur with this lock held
|
||||
* for write.
|
||||
*/
|
||||
DECLARE_RWSEM(dax_dev_rwsem);
|
||||
|
||||
#define DAX_NAME_LEN 30
|
||||
struct dax_id {
|
||||
struct list_head list;
|
||||
@ -180,7 +192,7 @@ static u64 dev_dax_size(struct dev_dax *dev_dax)
|
||||
u64 size = 0;
|
||||
int i;
|
||||
|
||||
device_lock_assert(&dev_dax->dev);
|
||||
WARN_ON_ONCE(!rwsem_is_locked(&dax_dev_rwsem));
|
||||
|
||||
for (i = 0; i < dev_dax->nr_range; i++)
|
||||
size += range_len(&dev_dax->ranges[i].range);
|
||||
@ -194,8 +206,15 @@ static int dax_bus_probe(struct device *dev)
|
||||
struct dev_dax *dev_dax = to_dev_dax(dev);
|
||||
struct dax_region *dax_region = dev_dax->region;
|
||||
int rc;
|
||||
u64 size;
|
||||
|
||||
if (dev_dax_size(dev_dax) == 0 || dev_dax->id < 0)
|
||||
rc = down_read_interruptible(&dax_dev_rwsem);
|
||||
if (rc)
|
||||
return rc;
|
||||
size = dev_dax_size(dev_dax);
|
||||
up_read(&dax_dev_rwsem);
|
||||
|
||||
if (size == 0 || dev_dax->id < 0)
|
||||
return -ENXIO;
|
||||
|
||||
rc = dax_drv->probe(dev_dax);
|
||||
@ -283,7 +302,7 @@ static unsigned long long dax_region_avail_size(struct dax_region *dax_region)
|
||||
resource_size_t size = resource_size(&dax_region->res);
|
||||
struct resource *res;
|
||||
|
||||
device_lock_assert(dax_region->dev);
|
||||
WARN_ON_ONCE(!rwsem_is_locked(&dax_region_rwsem));
|
||||
|
||||
for_each_dax_region_resource(dax_region, res)
|
||||
size -= resource_size(res);
|
||||
@ -295,10 +314,13 @@ static ssize_t available_size_show(struct device *dev,
|
||||
{
|
||||
struct dax_region *dax_region = dev_get_drvdata(dev);
|
||||
unsigned long long size;
|
||||
int rc;
|
||||
|
||||
device_lock(dev);
|
||||
rc = down_read_interruptible(&dax_region_rwsem);
|
||||
if (rc)
|
||||
return rc;
|
||||
size = dax_region_avail_size(dax_region);
|
||||
device_unlock(dev);
|
||||
up_read(&dax_region_rwsem);
|
||||
|
||||
return sprintf(buf, "%llu\n", size);
|
||||
}
|
||||
@ -314,10 +336,12 @@ static ssize_t seed_show(struct device *dev,
|
||||
if (is_static(dax_region))
|
||||
return -EINVAL;
|
||||
|
||||
device_lock(dev);
|
||||
rc = down_read_interruptible(&dax_region_rwsem);
|
||||
if (rc)
|
||||
return rc;
|
||||
seed = dax_region->seed;
|
||||
rc = sprintf(buf, "%s\n", seed ? dev_name(seed) : "");
|
||||
device_unlock(dev);
|
||||
up_read(&dax_region_rwsem);
|
||||
|
||||
return rc;
|
||||
}
|
||||
@ -333,14 +357,18 @@ static ssize_t create_show(struct device *dev,
|
||||
if (is_static(dax_region))
|
||||
return -EINVAL;
|
||||
|
||||
device_lock(dev);
|
||||
rc = down_read_interruptible(&dax_region_rwsem);
|
||||
if (rc)
|
||||
return rc;
|
||||
youngest = dax_region->youngest;
|
||||
rc = sprintf(buf, "%s\n", youngest ? dev_name(youngest) : "");
|
||||
device_unlock(dev);
|
||||
up_read(&dax_region_rwsem);
|
||||
|
||||
return rc;
|
||||
}
|
||||
|
||||
static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data);
|
||||
|
||||
static ssize_t create_store(struct device *dev, struct device_attribute *attr,
|
||||
const char *buf, size_t len)
|
||||
{
|
||||
@ -358,7 +386,9 @@ static ssize_t create_store(struct device *dev, struct device_attribute *attr,
|
||||
if (val != 1)
|
||||
return -EINVAL;
|
||||
|
||||
device_lock(dev);
|
||||
rc = down_write_killable(&dax_region_rwsem);
|
||||
if (rc)
|
||||
return rc;
|
||||
avail = dax_region_avail_size(dax_region);
|
||||
if (avail == 0)
|
||||
rc = -ENOSPC;
|
||||
@ -369,7 +399,7 @@ static ssize_t create_store(struct device *dev, struct device_attribute *attr,
|
||||
.id = -1,
|
||||
.memmap_on_memory = false,
|
||||
};
|
||||
struct dev_dax *dev_dax = devm_create_dev_dax(&data);
|
||||
struct dev_dax *dev_dax = __devm_create_dev_dax(&data);
|
||||
|
||||
if (IS_ERR(dev_dax))
|
||||
rc = PTR_ERR(dev_dax);
|
||||
@ -387,7 +417,7 @@ static ssize_t create_store(struct device *dev, struct device_attribute *attr,
|
||||
rc = len;
|
||||
}
|
||||
}
|
||||
device_unlock(dev);
|
||||
up_write(&dax_region_rwsem);
|
||||
|
||||
return rc;
|
||||
}
|
||||
@ -417,7 +447,7 @@ static void trim_dev_dax_range(struct dev_dax *dev_dax)
|
||||
struct range *range = &dev_dax->ranges[i].range;
|
||||
struct dax_region *dax_region = dev_dax->region;
|
||||
|
||||
device_lock_assert(dax_region->dev);
|
||||
WARN_ON_ONCE(!rwsem_is_locked(&dax_region_rwsem));
|
||||
dev_dbg(&dev_dax->dev, "delete range[%d]: %#llx:%#llx\n", i,
|
||||
(unsigned long long)range->start,
|
||||
(unsigned long long)range->end);
|
||||
@ -435,7 +465,7 @@ 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);
|
||||
|
||||
@ -447,6 +477,17 @@ static void unregister_dev_dax(void *dev)
|
||||
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);
|
||||
}
|
||||
|
||||
static void dax_region_free(struct kref *kref)
|
||||
{
|
||||
struct dax_region *dax_region;
|
||||
@ -463,11 +504,10 @@ static void dax_region_put(struct dax_region *dax_region)
|
||||
/* a return value >= 0 indicates this invocation invalidated the id */
|
||||
static int __free_dev_dax_id(struct dev_dax *dev_dax)
|
||||
{
|
||||
struct device *dev = &dev_dax->dev;
|
||||
struct dax_region *dax_region;
|
||||
int rc = dev_dax->id;
|
||||
|
||||
device_lock_assert(dev);
|
||||
WARN_ON_ONCE(!rwsem_is_locked(&dax_dev_rwsem));
|
||||
|
||||
if (!dev_dax->dyn_id || dev_dax->id < 0)
|
||||
return -1;
|
||||
@ -480,12 +520,13 @@ static int __free_dev_dax_id(struct dev_dax *dev_dax)
|
||||
|
||||
static int free_dev_dax_id(struct dev_dax *dev_dax)
|
||||
{
|
||||
struct device *dev = &dev_dax->dev;
|
||||
int rc;
|
||||
|
||||
device_lock(dev);
|
||||
rc = down_write_killable(&dax_dev_rwsem);
|
||||
if (rc)
|
||||
return rc;
|
||||
rc = __free_dev_dax_id(dev_dax);
|
||||
device_unlock(dev);
|
||||
up_write(&dax_dev_rwsem);
|
||||
return rc;
|
||||
}
|
||||
|
||||
@ -519,8 +560,14 @@ static ssize_t delete_store(struct device *dev, struct device_attribute *attr,
|
||||
if (!victim)
|
||||
return -ENXIO;
|
||||
|
||||
device_lock(dev);
|
||||
device_lock(victim);
|
||||
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;
|
||||
}
|
||||
dev_dax = to_dev_dax(victim);
|
||||
if (victim->driver || dev_dax_size(dev_dax))
|
||||
rc = -EBUSY;
|
||||
@ -541,12 +588,12 @@ static ssize_t delete_store(struct device *dev, struct device_attribute *attr,
|
||||
} else
|
||||
rc = -EBUSY;
|
||||
}
|
||||
device_unlock(victim);
|
||||
up_write(&dax_dev_rwsem);
|
||||
|
||||
/* won the race to invalidate the device, clean it up */
|
||||
if (do_del)
|
||||
devm_release_action(dev, unregister_dev_dax, victim);
|
||||
device_unlock(dev);
|
||||
up_write(&dax_region_rwsem);
|
||||
put_device(victim);
|
||||
|
||||
return rc;
|
||||
@ -658,16 +705,15 @@ 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);
|
||||
struct dev_dax *dev_dax = to_dev_dax(dev->parent);
|
||||
struct dax_region *dax_region = dev_dax->region;
|
||||
|
||||
dev_dbg(dev, "%s\n", __func__);
|
||||
|
||||
device_lock_assert(dax_region->dev);
|
||||
WARN_ON_ONCE(!rwsem_is_locked(&dax_region_rwsem));
|
||||
|
||||
dev_dax->ranges[mapping->range_id].mapping = NULL;
|
||||
mapping->range_id = -1;
|
||||
@ -675,28 +721,37 @@ static void unregister_dax_mapping(void *data)
|
||||
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);
|
||||
struct dev_dax *dev_dax = to_dev_dax(dev->parent);
|
||||
struct dax_region *dax_region = dev_dax->region;
|
||||
int rc;
|
||||
|
||||
device_lock(dax_region->dev);
|
||||
rc = down_write_killable(&dax_region_rwsem);
|
||||
if (rc)
|
||||
return NULL;
|
||||
if (mapping->range_id < 0) {
|
||||
device_unlock(dax_region->dev);
|
||||
up_write(&dax_region_rwsem);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
return &dev_dax->ranges[mapping->range_id];
|
||||
}
|
||||
|
||||
static void put_dax_range(struct dev_dax_range *dax_range)
|
||||
static void put_dax_range(void)
|
||||
{
|
||||
struct dax_mapping *mapping = dax_range->mapping;
|
||||
struct dev_dax *dev_dax = to_dev_dax(mapping->dev.parent);
|
||||
struct dax_region *dax_region = dev_dax->region;
|
||||
|
||||
device_unlock(dax_region->dev);
|
||||
up_write(&dax_region_rwsem);
|
||||
}
|
||||
|
||||
static ssize_t start_show(struct device *dev,
|
||||
@ -709,7 +764,7 @@ static ssize_t start_show(struct device *dev,
|
||||
if (!dax_range)
|
||||
return -ENXIO;
|
||||
rc = sprintf(buf, "%#llx\n", dax_range->range.start);
|
||||
put_dax_range(dax_range);
|
||||
put_dax_range();
|
||||
|
||||
return rc;
|
||||
}
|
||||
@ -725,7 +780,7 @@ static ssize_t end_show(struct device *dev,
|
||||
if (!dax_range)
|
||||
return -ENXIO;
|
||||
rc = sprintf(buf, "%#llx\n", dax_range->range.end);
|
||||
put_dax_range(dax_range);
|
||||
put_dax_range();
|
||||
|
||||
return rc;
|
||||
}
|
||||
@ -741,7 +796,7 @@ static ssize_t pgoff_show(struct device *dev,
|
||||
if (!dax_range)
|
||||
return -ENXIO;
|
||||
rc = sprintf(buf, "%#lx\n", dax_range->pgoff);
|
||||
put_dax_range(dax_range);
|
||||
put_dax_range();
|
||||
|
||||
return rc;
|
||||
}
|
||||
@ -775,7 +830,7 @@ static int devm_register_dax_mapping(struct dev_dax *dev_dax, int range_id)
|
||||
struct device *dev;
|
||||
int rc;
|
||||
|
||||
device_lock_assert(dax_region->dev);
|
||||
WARN_ON_ONCE(!rwsem_is_locked(&dax_region_rwsem));
|
||||
|
||||
if (dev_WARN_ONCE(&dev_dax->dev, !dax_region->dev->driver,
|
||||
"region disabled\n"))
|
||||
@ -821,7 +876,7 @@ static int alloc_dev_dax_range(struct dev_dax *dev_dax, u64 start,
|
||||
struct resource *alloc;
|
||||
int i, rc;
|
||||
|
||||
device_lock_assert(dax_region->dev);
|
||||
WARN_ON_ONCE(!rwsem_is_locked(&dax_region_rwsem));
|
||||
|
||||
/* handle the seed alloc special case */
|
||||
if (!size) {
|
||||
@ -875,13 +930,12 @@ static int adjust_dev_dax_range(struct dev_dax *dev_dax, struct resource *res, r
|
||||
{
|
||||
int last_range = dev_dax->nr_range - 1;
|
||||
struct dev_dax_range *dax_range = &dev_dax->ranges[last_range];
|
||||
struct dax_region *dax_region = dev_dax->region;
|
||||
bool is_shrink = resource_size(res) > size;
|
||||
struct range *range = &dax_range->range;
|
||||
struct device *dev = &dev_dax->dev;
|
||||
int rc;
|
||||
|
||||
device_lock_assert(dax_region->dev);
|
||||
WARN_ON_ONCE(!rwsem_is_locked(&dax_region_rwsem));
|
||||
|
||||
if (dev_WARN_ONCE(dev, !size, "deletion is handled by dev_dax_shrink\n"))
|
||||
return -EINVAL;
|
||||
@ -907,10 +961,13 @@ static ssize_t size_show(struct device *dev,
|
||||
{
|
||||
struct dev_dax *dev_dax = to_dev_dax(dev);
|
||||
unsigned long long size;
|
||||
int rc;
|
||||
|
||||
device_lock(dev);
|
||||
rc = down_write_killable(&dax_dev_rwsem);
|
||||
if (rc)
|
||||
return rc;
|
||||
size = dev_dax_size(dev_dax);
|
||||
device_unlock(dev);
|
||||
up_write(&dax_dev_rwsem);
|
||||
|
||||
return sprintf(buf, "%llu\n", size);
|
||||
}
|
||||
@ -1080,17 +1137,27 @@ static ssize_t size_store(struct device *dev, struct device_attribute *attr,
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
device_lock(dax_region->dev);
|
||||
rc = down_write_killable(&dax_region_rwsem);
|
||||
if (rc)
|
||||
return rc;
|
||||
if (!dax_region->dev->driver) {
|
||||
device_unlock(dax_region->dev);
|
||||
return -ENXIO;
|
||||
rc = -ENXIO;
|
||||
goto err_region;
|
||||
}
|
||||
device_lock(dev);
|
||||
rc = dev_dax_resize(dax_region, dev_dax, val);
|
||||
device_unlock(dev);
|
||||
device_unlock(dax_region->dev);
|
||||
rc = down_write_killable(&dax_dev_rwsem);
|
||||
if (rc)
|
||||
goto err_dev;
|
||||
|
||||
return rc == 0 ? len : rc;
|
||||
rc = dev_dax_resize(dax_region, dev_dax, val);
|
||||
|
||||
err_dev:
|
||||
up_write(&dax_dev_rwsem);
|
||||
err_region:
|
||||
up_write(&dax_region_rwsem);
|
||||
|
||||
if (rc == 0)
|
||||
return len;
|
||||
return rc;
|
||||
}
|
||||
static DEVICE_ATTR_RW(size);
|
||||
|
||||
@ -1138,18 +1205,24 @@ static ssize_t mapping_store(struct device *dev, struct device_attribute *attr,
|
||||
return rc;
|
||||
|
||||
rc = -ENXIO;
|
||||
device_lock(dax_region->dev);
|
||||
rc = down_write_killable(&dax_region_rwsem);
|
||||
if (rc)
|
||||
return rc;
|
||||
if (!dax_region->dev->driver) {
|
||||
device_unlock(dax_region->dev);
|
||||
up_write(&dax_region_rwsem);
|
||||
return rc;
|
||||
}
|
||||
rc = down_write_killable(&dax_dev_rwsem);
|
||||
if (rc) {
|
||||
up_write(&dax_region_rwsem);
|
||||
return rc;
|
||||
}
|
||||
device_lock(dev);
|
||||
|
||||
to_alloc = range_len(&r);
|
||||
if (alloc_is_aligned(dev_dax, to_alloc))
|
||||
rc = alloc_dev_dax_range(dev_dax, r.start, to_alloc);
|
||||
device_unlock(dev);
|
||||
device_unlock(dax_region->dev);
|
||||
up_write(&dax_dev_rwsem);
|
||||
up_write(&dax_region_rwsem);
|
||||
|
||||
return rc == 0 ? len : rc;
|
||||
}
|
||||
@ -1196,13 +1269,19 @@ static ssize_t align_store(struct device *dev, struct device_attribute *attr,
|
||||
if (!dax_align_valid(val))
|
||||
return -EINVAL;
|
||||
|
||||
device_lock(dax_region->dev);
|
||||
rc = down_write_killable(&dax_region_rwsem);
|
||||
if (rc)
|
||||
return rc;
|
||||
if (!dax_region->dev->driver) {
|
||||
device_unlock(dax_region->dev);
|
||||
up_write(&dax_region_rwsem);
|
||||
return -ENXIO;
|
||||
}
|
||||
|
||||
device_lock(dev);
|
||||
rc = down_write_killable(&dax_dev_rwsem);
|
||||
if (rc) {
|
||||
up_write(&dax_region_rwsem);
|
||||
return rc;
|
||||
}
|
||||
if (dev->driver) {
|
||||
rc = -EBUSY;
|
||||
goto out_unlock;
|
||||
@ -1214,8 +1293,8 @@ static ssize_t align_store(struct device *dev, struct device_attribute *attr,
|
||||
if (rc)
|
||||
dev_dax->align = align_save;
|
||||
out_unlock:
|
||||
device_unlock(dev);
|
||||
device_unlock(dax_region->dev);
|
||||
up_write(&dax_dev_rwsem);
|
||||
up_write(&dax_region_rwsem);
|
||||
return rc == 0 ? len : rc;
|
||||
}
|
||||
static DEVICE_ATTR_RW(align);
|
||||
@ -1325,7 +1404,7 @@ static const struct device_type dev_dax_type = {
|
||||
.groups = dax_attribute_groups,
|
||||
};
|
||||
|
||||
struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data)
|
||||
static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
|
||||
{
|
||||
struct dax_region *dax_region = data->dax_region;
|
||||
struct device *parent = dax_region->dev;
|
||||
@ -1440,6 +1519,21 @@ err_id:
|
||||
|
||||
return ERR_PTR(rc);
|
||||
}
|
||||
|
||||
struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data)
|
||||
{
|
||||
struct dev_dax *dev_dax;
|
||||
int rc;
|
||||
|
||||
rc = down_write_killable(&dax_region_rwsem);
|
||||
if (rc)
|
||||
return ERR_PTR(rc);
|
||||
|
||||
dev_dax = __devm_create_dev_dax(data);
|
||||
up_write(&dax_region_rwsem);
|
||||
|
||||
return dev_dax;
|
||||
}
|
||||
EXPORT_SYMBOL_GPL(devm_create_dev_dax);
|
||||
|
||||
int __dax_driver_register(struct dax_device_driver *dax_drv,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user