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.
---
Changes in v7:
- Rebase to v6.8-rc1
- Remove an unnecessary 'size' variable. (Matthew)
- Replace device lock (ab)use in dax/bus.c with local rwsems (Greg)
- Replace sprintf() usage with sysfs_emit() (Greg)
- Link to v6: https://lore.kernel.org/r/[email protected]
Changes in v6:
- Use sysfs_emit() in memmap_on_memory_show() (Greg)
- Change the ABI documentation date for memmap_on_memory to January 2024
as that's likely when the 6.8 merge window will fall (Greg)
- Fix dev->driver check (Ying)
- Link to v5: https://lore.kernel.org/r/[email protected]
Changes in v5:
- Export and check mhp_supports_memmap_on_memory() in the DAX sysfs ABI
(David)
- Obtain dax_drv under the device lock (Ying)
- Check dax_drv for NULL before dereferencing it (Ying)
- Clean up some repetition in sysfs-bus-dax documentation entries
(Jonathan)
- A few additional cleanups enabled by guard(device) (Jonathan)
- Drop the DEFINE_GUARD() part of patch 2, add dependency on Dan's patch
above so it can be backported / applied separately (Jonathan, Dan)
- Link to v4: https://lore.kernel.org/r/[email protected]
Changes in v4:
- Hold the device lock when checking if the dax_dev is bound to kmem
(Ying, Dan)
- Remove dax region checks (and locks) as they were unnecessary.
- Introduce guard(device) for device lock/unlock (Dan)
- Convert the rest of drivers/dax/bus.c to guard(device)
- Link to v3: https://lore.kernel.org/r/[email protected]
Changes in v3:
- Fix typo in ABI docs (Zhijian Li)
- Add kernel config and module parameter dependencies to the ABI docs
entry (David Hildenbrand)
- Ensure kmem isn't active when setting the sysfs attribute (Ying
Huang)
- Simplify returning from memmap_on_memory_store()
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- Fix CC lists, patch 1/2 didn't get sent correctly in v1
- Link to v1: https://lore.kernel.org/r/[email protected]
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: David Hildenbrand <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Huang Ying <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: <[email protected]>
To: Dan Williams <[email protected]>
To: Vishal Verma <[email protected]>
To: Dave Jiang <[email protected]>
To: Andrew Morton <[email protected]>
To: Oscar Salvador <[email protected]>
---
Vishal Verma (5):
dax/bus.c: replace driver-core lock usage by a local rwsem
dax/bus.c: replace several sprintf() with sysfs_emit()
Documentatiion/ABI: Add ABI documentation for sys-bus-dax
mm/memory_hotplug: export mhp_supports_memmap_on_memory()
dax: add a sysfs knob to control memmap_on_memory behavior
include/linux/memory_hotplug.h | 6 +
drivers/dax/bus.c | 295 +++++++++++++++++++++++---------
mm/memory_hotplug.c | 17 +-
Documentation/ABI/testing/sysfs-bus-dax | 153 +++++++++++++++++
4 files changed, 381 insertions(+), 90 deletions(-)
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20231025-vv-dax_abi-17a219c46076
Best regards,
--
Vishal Verma <[email protected]>
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.
Cc: Dan Williams <[email protected]>
Reported-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Vishal Verma <[email protected]>
---
drivers/dax/bus.c | 220 ++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 157 insertions(+), 63 deletions(-)
diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index 1ff1ab5fa105..cb148f74ceda 100644
--- a/drivers/dax/bus.c
+++ b/drivers/dax/bus.c
@@ -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 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data)
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,
--
2.43.0
There were several places where drivers/dax/bus.c uses 'sprintf' to
print sysfs data. Since a sysfs_emit() helper is available specifically
for this purpose, replace all the sprintf() usage for sysfs with
sysfs_emit() in this file.
Cc: Dan Williams <[email protected]>
Reported-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Vishal Verma <[email protected]>
---
drivers/dax/bus.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index cb148f74ceda..0fd948a4443e 100644
--- a/drivers/dax/bus.c
+++ b/drivers/dax/bus.c
@@ -269,7 +269,7 @@ static ssize_t id_show(struct device *dev,
{
struct dax_region *dax_region = dev_get_drvdata(dev);
- return sprintf(buf, "%d\n", dax_region->id);
+ return sysfs_emit(buf, "%d\n", dax_region->id);
}
static DEVICE_ATTR_RO(id);
@@ -278,8 +278,8 @@ static ssize_t region_size_show(struct device *dev,
{
struct dax_region *dax_region = dev_get_drvdata(dev);
- return sprintf(buf, "%llu\n", (unsigned long long)
- resource_size(&dax_region->res));
+ return sysfs_emit(buf, "%llu\n",
+ (unsigned long long)resource_size(&dax_region->res));
}
static struct device_attribute dev_attr_region_size = __ATTR(size, 0444,
region_size_show, NULL);
@@ -289,7 +289,7 @@ static ssize_t region_align_show(struct device *dev,
{
struct dax_region *dax_region = dev_get_drvdata(dev);
- return sprintf(buf, "%u\n", dax_region->align);
+ return sysfs_emit(buf, "%u\n", dax_region->align);
}
static struct device_attribute dev_attr_region_align =
__ATTR(align, 0400, region_align_show, NULL);
@@ -322,7 +322,7 @@ static ssize_t available_size_show(struct device *dev,
size = dax_region_avail_size(dax_region);
up_read(&dax_region_rwsem);
- return sprintf(buf, "%llu\n", size);
+ return sysfs_emit(buf, "%llu\n", size);
}
static DEVICE_ATTR_RO(available_size);
@@ -340,7 +340,7 @@ static ssize_t seed_show(struct device *dev,
if (rc)
return rc;
seed = dax_region->seed;
- rc = sprintf(buf, "%s\n", seed ? dev_name(seed) : "");
+ rc = sysfs_emit(buf, "%s\n", seed ? dev_name(seed) : "");
up_read(&dax_region_rwsem);
return rc;
@@ -361,7 +361,7 @@ static ssize_t create_show(struct device *dev,
if (rc)
return rc;
youngest = dax_region->youngest;
- rc = sprintf(buf, "%s\n", youngest ? dev_name(youngest) : "");
+ rc = sysfs_emit(buf, "%s\n", youngest ? dev_name(youngest) : "");
up_read(&dax_region_rwsem);
return rc;
@@ -763,7 +763,7 @@ static ssize_t start_show(struct device *dev,
dax_range = get_dax_range(dev);
if (!dax_range)
return -ENXIO;
- rc = sprintf(buf, "%#llx\n", dax_range->range.start);
+ rc = sysfs_emit(buf, "%#llx\n", dax_range->range.start);
put_dax_range();
return rc;
@@ -779,7 +779,7 @@ static ssize_t end_show(struct device *dev,
dax_range = get_dax_range(dev);
if (!dax_range)
return -ENXIO;
- rc = sprintf(buf, "%#llx\n", dax_range->range.end);
+ rc = sysfs_emit(buf, "%#llx\n", dax_range->range.end);
put_dax_range();
return rc;
@@ -795,7 +795,7 @@ static ssize_t pgoff_show(struct device *dev,
dax_range = get_dax_range(dev);
if (!dax_range)
return -ENXIO;
- rc = sprintf(buf, "%#lx\n", dax_range->pgoff);
+ rc = sysfs_emit(buf, "%#lx\n", dax_range->pgoff);
put_dax_range();
return rc;
@@ -969,7 +969,7 @@ static ssize_t size_show(struct device *dev,
size = dev_dax_size(dev_dax);
up_write(&dax_dev_rwsem);
- return sprintf(buf, "%llu\n", size);
+ return sysfs_emit(buf, "%llu\n", size);
}
static bool alloc_is_aligned(struct dev_dax *dev_dax, resource_size_t size)
@@ -1233,7 +1233,7 @@ static ssize_t align_show(struct device *dev,
{
struct dev_dax *dev_dax = to_dev_dax(dev);
- return sprintf(buf, "%d\n", dev_dax->align);
+ return sysfs_emit(buf, "%d\n", dev_dax->align);
}
static ssize_t dev_dax_validate_align(struct dev_dax *dev_dax)
@@ -1311,7 +1311,7 @@ static ssize_t target_node_show(struct device *dev,
{
struct dev_dax *dev_dax = to_dev_dax(dev);
- return sprintf(buf, "%d\n", dev_dax_target_node(dev_dax));
+ return sysfs_emit(buf, "%d\n", dev_dax_target_node(dev_dax));
}
static DEVICE_ATTR_RO(target_node);
@@ -1327,7 +1327,7 @@ static ssize_t resource_show(struct device *dev,
else
start = dev_dax->ranges[0].range.start;
- return sprintf(buf, "%#llx\n", start);
+ return sysfs_emit(buf, "%#llx\n", start);
}
static DEVICE_ATTR(resource, 0400, resource_show, NULL);
@@ -1338,14 +1338,14 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
* We only ever expect to handle device-dax instances, i.e. the
* @type argument to MODULE_ALIAS_DAX_DEVICE() is always zero
*/
- return sprintf(buf, DAX_DEVICE_MODALIAS_FMT "\n", 0);
+ return sysfs_emit(buf, DAX_DEVICE_MODALIAS_FMT "\n", 0);
}
static DEVICE_ATTR_RO(modalias);
static ssize_t numa_node_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "%d\n", dev_to_node(dev));
+ return sysfs_emit(buf, "%d\n", dev_to_node(dev));
}
static DEVICE_ATTR_RO(numa_node);
--
2.43.0
Add the missing sysfs ABI documentation for the device DAX subsystem.
Various ABI attributes under this have been present since v5.1, and more
have been added over time. In preparation for adding a new attribute,
add this file with the historical details.
Cc: Dan Williams <[email protected]>
Signed-off-by: Vishal Verma <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-dax | 136 ++++++++++++++++++++++++++++++++
1 file changed, 136 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-dax b/Documentation/ABI/testing/sysfs-bus-dax
new file mode 100644
index 000000000000..6359f7bc9bf4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-dax
@@ -0,0 +1,136 @@
+What: /sys/bus/dax/devices/daxX.Y/align
+Date: October, 2020
+KernelVersion: v5.10
+Contact: [email protected]
+Description:
+ (RW) Provides a way to specify an alignment for a dax device.
+ Values allowed are constrained by the physical address ranges
+ that back the dax device, and also by arch requirements.
+
+What: /sys/bus/dax/devices/daxX.Y/mapping
+Date: October, 2020
+KernelVersion: v5.10
+Contact: [email protected]
+Description:
+ (WO) Provides a way to allocate a mapping range under a dax
+ device. Specified in the format <start>-<end>.
+
+What: /sys/bus/dax/devices/daxX.Y/mapping[0..N]/start
+What: /sys/bus/dax/devices/daxX.Y/mapping[0..N]/end
+What: /sys/bus/dax/devices/daxX.Y/mapping[0..N]/page_offset
+Date: October, 2020
+KernelVersion: v5.10
+Contact: [email protected]
+Description:
+ (RO) A dax device may have multiple constituent discontiguous
+ address ranges. These are represented by the different
+ 'mappingX' subdirectories. The 'start' attribute indicates the
+ start physical address for the given range. The 'end' attribute
+ indicates the end physical address for the given range. The
+ 'page_offset' attribute indicates the offset of the current
+ range in the dax device.
+
+What: /sys/bus/dax/devices/daxX.Y/resource
+Date: June, 2019
+KernelVersion: v5.3
+Contact: [email protected]
+Description:
+ (RO) The resource attribute indicates the starting physical
+ address of a dax device. In case of a device with multiple
+ constituent ranges, it indicates the starting address of the
+ first range.
+
+What: /sys/bus/dax/devices/daxX.Y/size
+Date: October, 2020
+KernelVersion: v5.10
+Contact: [email protected]
+Description:
+ (RW) The size attribute indicates the total size of a dax
+ device. For creating subdivided dax devices, or for resizing
+ an existing device, the new size can be written to this as
+ part of the reconfiguration process.
+
+What: /sys/bus/dax/devices/daxX.Y/numa_node
+Date: November, 2019
+KernelVersion: v5.5
+Contact: [email protected]
+Description:
+ (RO) If NUMA is enabled and the platform has affinitized the
+ backing device for this dax device, emit the CPU node
+ affinity for this device.
+
+What: /sys/bus/dax/devices/daxX.Y/target_node
+Date: February, 2019
+KernelVersion: v5.1
+Contact: [email protected]
+Description:
+ (RO) The target-node attribute is the Linux numa-node that a
+ device-dax instance may create when it is online. Prior to
+ being online the device's 'numa_node' property reflects the
+ closest online cpu node which is the typical expectation of a
+ device 'numa_node'. Once it is online it becomes its own
+ distinct numa node.
+
+What: $(readlink -f /sys/bus/dax/devices/daxX.Y)/../dax_region/available_size
+Date: October, 2020
+KernelVersion: v5.10
+Contact: [email protected]
+Description:
+ (RO) The available_size attribute tracks available dax region
+ capacity. This only applies to volatile hmem devices, not pmem
+ devices, since pmem devices are defined by nvdimm namespace
+ boundaries.
+
+What: $(readlink -f /sys/bus/dax/devices/daxX.Y)/../dax_region/size
+Date: July, 2017
+KernelVersion: v5.1
+Contact: [email protected]
+Description:
+ (RO) The size attribute indicates the size of a given dax region
+ in bytes.
+
+What: $(readlink -f /sys/bus/dax/devices/daxX.Y)/../dax_region/align
+Date: October, 2020
+KernelVersion: v5.10
+Contact: [email protected]
+Description:
+ (RO) The align attribute indicates alignment of the dax region.
+ Changes on align may not always be valid, when say certain
+ mappings were created with 2M and then we switch to 1G. This
+ validates all ranges against the new value being attempted, post
+ resizing.
+
+What: $(readlink -f /sys/bus/dax/devices/daxX.Y)/../dax_region/seed
+Date: October, 2020
+KernelVersion: v5.10
+Contact: [email protected]
+Description:
+ (RO) The seed device is a concept for dynamic dax regions to be
+ able to split the region amongst multiple sub-instances. The
+ seed device, similar to libnvdimm seed devices, is a device
+ that starts with zero capacity allocated and unbound to a
+ driver.
+
+What: $(readlink -f /sys/bus/dax/devices/daxX.Y)/../dax_region/create
+Date: October, 2020
+KernelVersion: v5.10
+Contact: [email protected]
+Description:
+ (RW) The create interface to the dax region provides a way to
+ create a new unconfigured dax device under the given region, which
+ can then be configured (with a size etc.) and then probed.
+
+What: $(readlink -f /sys/bus/dax/devices/daxX.Y)/../dax_region/delete
+Date: October, 2020
+KernelVersion: v5.10
+Contact: [email protected]
+Description:
+ (WO) The delete interface for a dax region provides for deletion
+ of any 0-sized and idle dax devices.
+
+What: $(readlink -f /sys/bus/dax/devices/daxX.Y)/../dax_region/id
+Date: July, 2017
+KernelVersion: v5.1
+Contact: [email protected]
+Description:
+ (RO) The id attribute indicates the region id of a dax region.
--
2.43.0
In preparation for adding sysfs ABI to toggle memmap_on_memory semantics
for drivers adding memory, export the mhp_supports_memmap_on_memory()
helper. This allows drivers to check if memmap_on_memory support is
available before trying to request it, and display an appropriate
message if it isn't available. As part of this, remove the size argument
to this - with recent updates to allow memmap_on_memory for larger
ranges, and the internal splitting of altmaps into respective memory
blocks, the size argument is meaningless.
Cc: Andrew Morton <[email protected]>
Cc: David Hildenbrand <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Oscar Salvador <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Dave Jiang <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Huang Ying <[email protected]>
Suggested-by: David Hildenbrand <[email protected]>
Acked-by: David Hildenbrand <[email protected]>
Signed-off-by: Vishal Verma <[email protected]>
---
include/linux/memory_hotplug.h | 6 ++++++
mm/memory_hotplug.c | 17 ++++++-----------
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 7d2076583494..ebc9d528f00c 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -121,6 +121,7 @@ struct mhp_params {
bool mhp_range_allowed(u64 start, u64 size, bool need_mapping);
struct range mhp_get_pluggable_range(bool need_mapping);
+bool mhp_supports_memmap_on_memory(void);
/*
* Zone resizing functions
@@ -262,6 +263,11 @@ static inline bool movable_node_is_enabled(void)
return false;
}
+static bool mhp_supports_memmap_on_memory(void)
+{
+ return false;
+}
+
static inline void pgdat_kswapd_lock(pg_data_t *pgdat) {}
static inline void pgdat_kswapd_unlock(pg_data_t *pgdat) {}
static inline void pgdat_kswapd_lock_init(pg_data_t *pgdat) {}
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 21890994c1d3..065fb4804f1b 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1328,7 +1328,7 @@ static inline bool arch_supports_memmap_on_memory(unsigned long vmemmap_size)
}
#endif
-static bool mhp_supports_memmap_on_memory(unsigned long size)
+bool mhp_supports_memmap_on_memory(void)
{
unsigned long vmemmap_size = memory_block_memmap_size();
unsigned long memmap_pages = memory_block_memmap_on_memory_pages();
@@ -1337,17 +1337,11 @@ static bool mhp_supports_memmap_on_memory(unsigned long size)
* Besides having arch support and the feature enabled at runtime, we
* need a few more assumptions to hold true:
*
- * a) We span a single memory block: memory onlining/offlinin;g happens
- * in memory block granularity. We don't want the vmemmap of online
- * memory blocks to reside on offline memory blocks. In the future,
- * we might want to support variable-sized memory blocks to make the
- * feature more versatile.
- *
- * b) The vmemmap pages span complete PMDs: We don't want vmemmap code
+ * a) The vmemmap pages span complete PMDs: We don't want vmemmap code
* to populate memory from the altmap for unrelated parts (i.e.,
* other memory blocks)
*
- * c) The vmemmap pages (and thereby the pages that will be exposed to
+ * b) The vmemmap pages (and thereby the pages that will be exposed to
* the buddy) have to cover full pageblocks: memory onlining/offlining
* code requires applicable ranges to be page-aligned, for example, to
* set the migratetypes properly.
@@ -1359,7 +1353,7 @@ static bool mhp_supports_memmap_on_memory(unsigned long size)
* altmap as an alternative source of memory, and we do not exactly
* populate a single PMD.
*/
- if (!mhp_memmap_on_memory() || size != memory_block_size_bytes())
+ if (!mhp_memmap_on_memory())
return false;
/*
@@ -1382,6 +1376,7 @@ static bool mhp_supports_memmap_on_memory(unsigned long size)
return arch_supports_memmap_on_memory(vmemmap_size);
}
+EXPORT_SYMBOL_GPL(mhp_supports_memmap_on_memory);
static void __ref remove_memory_blocks_and_altmaps(u64 start, u64 size)
{
@@ -1515,7 +1510,7 @@ int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
* Self hosted memmap array
*/
if ((mhp_flags & MHP_MEMMAP_ON_MEMORY) &&
- mhp_supports_memmap_on_memory(memory_block_size_bytes())) {
+ mhp_supports_memmap_on_memory()) {
ret = create_altmaps_and_memory_blocks(nid, group, start, size);
if (ret)
goto error;
--
2.43.0
Add a sysfs knob for dax devices to control the memmap_on_memory setting
if the dax device were to be hotplugged as system memory.
The default memmap_on_memory setting for dax devices originating via
pmem or hmem is set to 'false' - i.e. no memmap_on_memory semantics, to
preserve legacy behavior. For dax devices via CXL, the default is on.
The sysfs control allows the administrator to override the above
defaults if needed.
Cc: David Hildenbrand <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Dave Jiang <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Huang Ying <[email protected]>
Tested-by: Li Zhijian <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Reviewed-by: David Hildenbrand <[email protected]>
Reviewed-by: Huang, Ying <[email protected]>
Signed-off-by: Vishal Verma <[email protected]>
---
drivers/dax/bus.c | 43 +++++++++++++++++++++++++++++++++
Documentation/ABI/testing/sysfs-bus-dax | 17 +++++++++++++
2 files changed, 60 insertions(+)
diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index 0fd948a4443e..27c86d0ca711 100644
--- a/drivers/dax/bus.c
+++ b/drivers/dax/bus.c
@@ -1349,6 +1349,48 @@ static ssize_t numa_node_show(struct device *dev,
}
static DEVICE_ATTR_RO(numa_node);
+static ssize_t memmap_on_memory_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct dev_dax *dev_dax = to_dev_dax(dev);
+
+ return sysfs_emit(buf, "%d\n", dev_dax->memmap_on_memory);
+}
+
+static ssize_t memmap_on_memory_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct dev_dax *dev_dax = to_dev_dax(dev);
+ bool val;
+ int rc;
+
+ rc = kstrtobool(buf, &val);
+ if (rc)
+ return rc;
+
+ if (val == true && !mhp_supports_memmap_on_memory()) {
+ dev_dbg(dev, "memmap_on_memory is not available\n");
+ return -EOPNOTSUPP;
+ }
+
+ rc = down_write_killable(&dax_dev_rwsem);
+ if (rc)
+ return rc;
+
+ if (dev_dax->memmap_on_memory != val && dev->driver &&
+ to_dax_drv(dev->driver)->type == DAXDRV_KMEM_TYPE) {
+ up_write(&dax_dev_rwsem);
+ return -EBUSY;
+ }
+
+ dev_dax->memmap_on_memory = val;
+ up_write(&dax_dev_rwsem);
+
+ return len;
+}
+static DEVICE_ATTR_RW(memmap_on_memory);
+
static umode_t dev_dax_visible(struct kobject *kobj, struct attribute *a, int n)
{
struct device *dev = container_of(kobj, struct device, kobj);
@@ -1375,6 +1417,7 @@ static struct attribute *dev_dax_attributes[] = {
&dev_attr_align.attr,
&dev_attr_resource.attr,
&dev_attr_numa_node.attr,
+ &dev_attr_memmap_on_memory.attr,
NULL,
};
diff --git a/Documentation/ABI/testing/sysfs-bus-dax b/Documentation/ABI/testing/sysfs-bus-dax
index 6359f7bc9bf4..b34266bfae49 100644
--- a/Documentation/ABI/testing/sysfs-bus-dax
+++ b/Documentation/ABI/testing/sysfs-bus-dax
@@ -134,3 +134,20 @@ KernelVersion: v5.1
Contact: [email protected]
Description:
(RO) The id attribute indicates the region id of a dax region.
+
+What: /sys/bus/dax/devices/daxX.Y/memmap_on_memory
+Date: January, 2024
+KernelVersion: v6.8
+Contact: [email protected]
+Description:
+ (RW) Control the memmap_on_memory setting if the dax device
+ were to be hotplugged as system memory. This determines whether
+ the 'altmap' for the hotplugged memory will be placed on the
+ device being hotplugged (memmap_on_memory=1) or if it will be
+ placed on regular memory (memmap_on_memory=0). This attribute
+ must be set before the device is handed over to the 'kmem'
+ driver (i.e. hotplugged into system-ram). Additionally, this
+ depends on CONFIG_MHP_MEMMAP_ON_MEMORY, and a globally enabled
+ memmap_on_memory parameter for memory_hotplug. This is
+ typically set on the kernel command line -
+ memory_hotplug.memmap_on_memory set to 'true' or 'force'."
--
2.43.0
On Wed, 24 Jan 2024 12:03:45 -0800 Vishal Verma <[email protected]> wrote:
> This series adds sysfs ABI to control memmap_on_memory behavior for DAX
> devices.
Thanks. I'll add this to mm-unstable for some additional testing, but
I do think we should have the evidence of additional review on this
series's four preparatory patches before proceeding further.
On Wed, Jan 24, 2024 at 12:03:46PM -0800, Vishal Verma wrote:
> 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.
>
Reviewed-by: Alison Schofield <[email protected]>
> Cc: Dan Williams <[email protected]>
> Reported-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Vishal Verma <[email protected]>
> ---
> drivers/dax/bus.c | 220 ++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 157 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 1ff1ab5fa105..cb148f74ceda 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -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 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data)
>
> 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,
>
> --
> 2.43.0
>
>
On Wed, Jan 24, 2024 at 12:03:47PM -0800, Vishal Verma wrote:
> There were several places where drivers/dax/bus.c uses 'sprintf' to
> print sysfs data. Since a sysfs_emit() helper is available specifically
> for this purpose, replace all the sprintf() usage for sysfs with
> sysfs_emit() in this file.
>
Reviewed-by: Alison Schofield <[email protected]>
> Cc: Dan Williams <[email protected]>
> Reported-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Vishal Verma <[email protected]>
> ---
> drivers/dax/bus.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index cb148f74ceda..0fd948a4443e 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -269,7 +269,7 @@ static ssize_t id_show(struct device *dev,
> {
> struct dax_region *dax_region = dev_get_drvdata(dev);
>
> - return sprintf(buf, "%d\n", dax_region->id);
> + return sysfs_emit(buf, "%d\n", dax_region->id);
> }
> static DEVICE_ATTR_RO(id);
>
> @@ -278,8 +278,8 @@ static ssize_t region_size_show(struct device *dev,
> {
> struct dax_region *dax_region = dev_get_drvdata(dev);
>
> - return sprintf(buf, "%llu\n", (unsigned long long)
> - resource_size(&dax_region->res));
> + return sysfs_emit(buf, "%llu\n",
> + (unsigned long long)resource_size(&dax_region->res));
> }
> static struct device_attribute dev_attr_region_size = __ATTR(size, 0444,
> region_size_show, NULL);
> @@ -289,7 +289,7 @@ static ssize_t region_align_show(struct device *dev,
> {
> struct dax_region *dax_region = dev_get_drvdata(dev);
>
> - return sprintf(buf, "%u\n", dax_region->align);
> + return sysfs_emit(buf, "%u\n", dax_region->align);
> }
> static struct device_attribute dev_attr_region_align =
> __ATTR(align, 0400, region_align_show, NULL);
> @@ -322,7 +322,7 @@ static ssize_t available_size_show(struct device *dev,
> size = dax_region_avail_size(dax_region);
> up_read(&dax_region_rwsem);
>
> - return sprintf(buf, "%llu\n", size);
> + return sysfs_emit(buf, "%llu\n", size);
> }
> static DEVICE_ATTR_RO(available_size);
>
> @@ -340,7 +340,7 @@ static ssize_t seed_show(struct device *dev,
> if (rc)
> return rc;
> seed = dax_region->seed;
> - rc = sprintf(buf, "%s\n", seed ? dev_name(seed) : "");
> + rc = sysfs_emit(buf, "%s\n", seed ? dev_name(seed) : "");
> up_read(&dax_region_rwsem);
>
> return rc;
> @@ -361,7 +361,7 @@ static ssize_t create_show(struct device *dev,
> if (rc)
> return rc;
> youngest = dax_region->youngest;
> - rc = sprintf(buf, "%s\n", youngest ? dev_name(youngest) : "");
> + rc = sysfs_emit(buf, "%s\n", youngest ? dev_name(youngest) : "");
> up_read(&dax_region_rwsem);
>
> return rc;
> @@ -763,7 +763,7 @@ static ssize_t start_show(struct device *dev,
> dax_range = get_dax_range(dev);
> if (!dax_range)
> return -ENXIO;
> - rc = sprintf(buf, "%#llx\n", dax_range->range.start);
> + rc = sysfs_emit(buf, "%#llx\n", dax_range->range.start);
> put_dax_range();
>
> return rc;
> @@ -779,7 +779,7 @@ static ssize_t end_show(struct device *dev,
> dax_range = get_dax_range(dev);
> if (!dax_range)
> return -ENXIO;
> - rc = sprintf(buf, "%#llx\n", dax_range->range.end);
> + rc = sysfs_emit(buf, "%#llx\n", dax_range->range.end);
> put_dax_range();
>
> return rc;
> @@ -795,7 +795,7 @@ static ssize_t pgoff_show(struct device *dev,
> dax_range = get_dax_range(dev);
> if (!dax_range)
> return -ENXIO;
> - rc = sprintf(buf, "%#lx\n", dax_range->pgoff);
> + rc = sysfs_emit(buf, "%#lx\n", dax_range->pgoff);
> put_dax_range();
>
> return rc;
> @@ -969,7 +969,7 @@ static ssize_t size_show(struct device *dev,
> size = dev_dax_size(dev_dax);
> up_write(&dax_dev_rwsem);
>
> - return sprintf(buf, "%llu\n", size);
> + return sysfs_emit(buf, "%llu\n", size);
> }
>
> static bool alloc_is_aligned(struct dev_dax *dev_dax, resource_size_t size)
> @@ -1233,7 +1233,7 @@ static ssize_t align_show(struct device *dev,
> {
> struct dev_dax *dev_dax = to_dev_dax(dev);
>
> - return sprintf(buf, "%d\n", dev_dax->align);
> + return sysfs_emit(buf, "%d\n", dev_dax->align);
> }
>
> static ssize_t dev_dax_validate_align(struct dev_dax *dev_dax)
> @@ -1311,7 +1311,7 @@ static ssize_t target_node_show(struct device *dev,
> {
> struct dev_dax *dev_dax = to_dev_dax(dev);
>
> - return sprintf(buf, "%d\n", dev_dax_target_node(dev_dax));
> + return sysfs_emit(buf, "%d\n", dev_dax_target_node(dev_dax));
> }
> static DEVICE_ATTR_RO(target_node);
>
> @@ -1327,7 +1327,7 @@ static ssize_t resource_show(struct device *dev,
> else
> start = dev_dax->ranges[0].range.start;
>
> - return sprintf(buf, "%#llx\n", start);
> + return sysfs_emit(buf, "%#llx\n", start);
> }
> static DEVICE_ATTR(resource, 0400, resource_show, NULL);
>
> @@ -1338,14 +1338,14 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> * We only ever expect to handle device-dax instances, i.e. the
> * @type argument to MODULE_ALIAS_DAX_DEVICE() is always zero
> */
> - return sprintf(buf, DAX_DEVICE_MODALIAS_FMT "\n", 0);
> + return sysfs_emit(buf, DAX_DEVICE_MODALIAS_FMT "\n", 0);
> }
> static DEVICE_ATTR_RO(modalias);
>
> static ssize_t numa_node_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return sprintf(buf, "%d\n", dev_to_node(dev));
> + return sysfs_emit(buf, "%d\n", dev_to_node(dev));
> }
> static DEVICE_ATTR_RO(numa_node);
>
>
> --
> 2.43.0
>
>
On Wed, Jan 24, 2024 at 12:03:50PM -0800, Vishal Verma wrote:
> Add a sysfs knob for dax devices to control the memmap_on_memory setting
> if the dax device were to be hotplugged as system memory.
>
> The default memmap_on_memory setting for dax devices originating via
> pmem or hmem is set to 'false' - i.e. no memmap_on_memory semantics, to
> preserve legacy behavior. For dax devices via CXL, the default is on.
> The sysfs control allows the administrator to override the above
> defaults if needed.
Reviewed-by: Alison Schofield <[email protected]>
>
> Cc: David Hildenbrand <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Dave Jiang <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Huang Ying <[email protected]>
> Tested-by: Li Zhijian <[email protected]>
> Reviewed-by: Jonathan Cameron <[email protected]>
> Reviewed-by: David Hildenbrand <[email protected]>
> Reviewed-by: Huang, Ying <[email protected]>
> Signed-off-by: Vishal Verma <[email protected]>
> ---
> drivers/dax/bus.c | 43 +++++++++++++++++++++++++++++++++
> Documentation/ABI/testing/sysfs-bus-dax | 17 +++++++++++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 0fd948a4443e..27c86d0ca711 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -1349,6 +1349,48 @@ static ssize_t numa_node_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(numa_node);
>
> +static ssize_t memmap_on_memory_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct dev_dax *dev_dax = to_dev_dax(dev);
> +
> + return sysfs_emit(buf, "%d\n", dev_dax->memmap_on_memory);
> +}
> +
> +static ssize_t memmap_on_memory_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct dev_dax *dev_dax = to_dev_dax(dev);
> + bool val;
> + int rc;
> +
> + rc = kstrtobool(buf, &val);
> + if (rc)
> + return rc;
> +
> + if (val == true && !mhp_supports_memmap_on_memory()) {
> + dev_dbg(dev, "memmap_on_memory is not available\n");
> + return -EOPNOTSUPP;
> + }
> +
> + rc = down_write_killable(&dax_dev_rwsem);
> + if (rc)
> + return rc;
> +
> + if (dev_dax->memmap_on_memory != val && dev->driver &&
> + to_dax_drv(dev->driver)->type == DAXDRV_KMEM_TYPE) {
> + up_write(&dax_dev_rwsem);
> + return -EBUSY;
> + }
> +
> + dev_dax->memmap_on_memory = val;
> + up_write(&dax_dev_rwsem);
> +
> + return len;
> +}
> +static DEVICE_ATTR_RW(memmap_on_memory);
> +
> static umode_t dev_dax_visible(struct kobject *kobj, struct attribute *a, int n)
> {
> struct device *dev = container_of(kobj, struct device, kobj);
> @@ -1375,6 +1417,7 @@ static struct attribute *dev_dax_attributes[] = {
> &dev_attr_align.attr,
> &dev_attr_resource.attr,
> &dev_attr_numa_node.attr,
> + &dev_attr_memmap_on_memory.attr,
> NULL,
> };
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-dax b/Documentation/ABI/testing/sysfs-bus-dax
> index 6359f7bc9bf4..b34266bfae49 100644
> --- a/Documentation/ABI/testing/sysfs-bus-dax
> +++ b/Documentation/ABI/testing/sysfs-bus-dax
> @@ -134,3 +134,20 @@ KernelVersion: v5.1
> Contact: [email protected]
> Description:
> (RO) The id attribute indicates the region id of a dax region.
> +
> +What: /sys/bus/dax/devices/daxX.Y/memmap_on_memory
> +Date: January, 2024
> +KernelVersion: v6.8
> +Contact: [email protected]
> +Description:
> + (RW) Control the memmap_on_memory setting if the dax device
> + were to be hotplugged as system memory. This determines whether
> + the 'altmap' for the hotplugged memory will be placed on the
> + device being hotplugged (memmap_on_memory=1) or if it will be
> + placed on regular memory (memmap_on_memory=0). This attribute
> + must be set before the device is handed over to the 'kmem'
> + driver (i.e. hotplugged into system-ram). Additionally, this
> + depends on CONFIG_MHP_MEMMAP_ON_MEMORY, and a globally enabled
> + memmap_on_memory parameter for memory_hotplug. This is
> + typically set on the kernel command line -
> + memory_hotplug.memmap_on_memory set to 'true' or 'force'."
>
> --
> 2.43.0
>
>
Vishal Verma wrote:
> 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.
>
> Cc: Dan Williams <[email protected]>
> Reported-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Vishal Verma <[email protected]>
> ---
> drivers/dax/bus.c | 220 ++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 157 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 1ff1ab5fa105..cb148f74ceda 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -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));
Apologies for the late review, but...
All of these WARN_ON_ONCE() usages should be replaced with
lockdep_assert_held() and lockdep_assert_held_write() where appropriate.
On Tue, 2024-03-12 at 13:07 -0700, Dan Williams wrote:
> Vishal Verma wrote:
> > 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.
> >
> > Cc: Dan Williams <[email protected]>
> > Reported-by: Greg Kroah-Hartman <[email protected]>
> > Signed-off-by: Vishal Verma <[email protected]>
> > ---
> > drivers/dax/bus.c | 220 ++++++++++++++++++++++++++++++++++++++----------------
> > 1 file changed, 157 insertions(+), 63 deletions(-)
> >
> > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> > index 1ff1ab5fa105..cb148f74ceda 100644
> > --- a/drivers/dax/bus.c
> > +++ b/drivers/dax/bus.c
> > @@ -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));
>
> Apologies for the late review, but...
>
> All of these WARN_ON_ONCE() usages should be replaced with
> lockdep_assert_held() and lockdep_assert_held_write() where appropriate.
Makes sense - I can send a patch post -rc1 to change these if that's okay Andrew?
On Tue, 12 Mar 2024 20:20:17 +0000 "Verma, Vishal L" <[email protected]> wrote:
> > All of these WARN_ON_ONCE() usages should be replaced with
> > lockdep_assert_held() and lockdep_assert_held_write() where appropriate.
>
> Makes sense - I can send a patch post -rc1 to change these if that's okay Andrew?
Please do.