Re: [PATCH v6 5/9] dax: Track all dax_region allocations under a global resource tree

Koralahalli Channabasappa, Smita
2026-03-12T21:30:36Z

Hi Jonathan,

On 3/9/2026 7:37 AM, Jonathan Cameron wrote:
> On Tue, 10 Feb 2026 06:44:57 +0000
> Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote:
>
>> Introduce a global "DAX Regions" resource root and register each
>> dax_region->res under it via request_resource(). Release the resource on
>> dax_region teardown.
>>
>> By enforcing a single global namespace for dax_region allocations, this
>> ensures only one of dax_hmem or dax_cxl can successfully register a
>> dax_region for a given range.
>>
>> Co-developed-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
>
> One question inline about the locking.
>
> Is intent to serialize beyond this new resource tree? If it's just
> the resource tree the write_lock(&resource_lock); in the request
> and release_resource() should be sufficient.
>
>> ---
>> drivers/dax/bus.c | 23 ++++++++++++++++++++---
>> 1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
>> index fde29e0ad68b..5f387feb95f0 100644
>> --- a/drivers/dax/bus.c
>> +++ b/drivers/dax/bus.c
>> @@ -10,6 +10,7 @@
>> #include "dax-private.h"
>> #include "bus.h"
>>
>> +static struct resource dax_regions = DEFINE_RES_MEM_NAMED(0, -1, "DAX Regions");
>> static DEFINE_MUTEX(dax_bus_lock);
>>
>> /*
>> @@ -625,6 +626,8 @@ static void dax_region_unregister(void *region)
>> {
>> struct dax_region *dax_region = region;
>>
>> + scoped_guard(rwsem_write, &dax_region_rwsem)
>> + release_resource(&dax_region->res);
>
> Do we need the locking? resource stuff all runs under the global
> resource_lock so if aim is just to serialize adds and removes that should
> be enough. Maybe there is a justification in that being an internal
> implementation detail.

Yeah the wrapping is unnecessary I will drop it.

Thanks
Smita

>
>
>
>> sysfs_remove_groups(&dax_region->dev->kobj,
>> dax_region_attribute_groups);
>> dax_region_put(dax_region);
>> @@ -635,6 +638,7 @@ struct dax_region *alloc_dax_region(struct device *parent, int region_id,
>> unsigned long flags)
>> {
>> struct dax_region *dax_region;
>> + int rc;
>>
>> /*
>> * The DAX core assumes that it can store its private data in
>> @@ -667,14 +671,27 @@ struct dax_region *alloc_dax_region(struct device *parent, int region_id,
>> .flags = IORESOURCE_MEM | flags,
>> };
>>
>> - if (sysfs_create_groups(&parent->kobj, dax_region_attribute_groups)) {
>> - kfree(dax_region);
>> - return NULL;
>> + scoped_guard(rwsem_write, &dax_region_rwsem)
>> + rc = request_resource(&dax_regions, &dax_region->res);
>> + if (rc) {
>> + dev_dbg(parent, "dax_region resource conflict for %pR\n",
>> + &dax_region->res);
>> + goto err_res;
>> }
>>
>> + if (sysfs_create_groups(&parent->kobj, dax_region_attribute_groups))
>> + goto err_sysfs;
>> +
>> if (devm_add_action_or_reset(parent, dax_region_unregister, dax_region))
>> return NULL;
>> return dax_region;
>> +
>> +err_sysfs:
>> + scoped_guard(rwsem_write, &dax_region_rwsem)
>> + release_resource(&dax_region->res);
>> +err_res:
>> + kfree(dax_region);
>> + return NULL;
>> }
>> EXPORT_SYMBOL_GPL(alloc_dax_region);
>>
>