Re: [PATCH v11 20/31] cxl/region/extent: Expose dc_extent information in sysfs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Double `xa_erase` race condition in `online_tag_group` error path can silently delete concurrently allocated extents.
- [High] Partial group accepted in `cxl_realize_group` on `cxl_add_extent` failure, violating More-chain atomicity.
--

commit 802d981e0cb1d1c5a24f7358b9d9626230786651
Author: Ira Weiny <iweiny@kernel.org>

cxl/region/extent: Expose dc_extent information in sysfs

Extent information can be helpful to the user to coordinate memory usage
with the external orchestrator and FM. Expose the details of each
dc_extent by creating sysfs entries.

> diff --git a/drivers/cxl/core/extent.c b/drivers/cxl/core/extent.c
> index 36be56ca10974..69c993cdd558b 100644
> --- a/drivers/cxl/core/extent.c
> +++ b/drivers/cxl/core/extent.c

[ ... ]

> @@ -99,6 +154,7 @@ static void dc_extent_release(struct device *dev)
>  static const struct device_type dc_extent_type = {
>  	.name = "extent",
>  	.release = dc_extent_release,
> +	.groups = dc_extent_attribute_groups,
>  };

[Severity: High]
This is a pre-existing issue, but does the error path in online_tag_group()
have a race condition that could silently delete concurrently allocated
extents?

If dev_set_name() or device_add() fails in online_tag_group(), the error path
calls xa_erase() followed by put_device():

drivers/cxl/core/extent.c:online_tag_group() {
    ...
		rc = dev_set_name(dev, "extent%d.%d", cxlr_dax->cxlr->id,
				  dev->id);
		if (rc) {
			xa_erase(&cxlr_dax->dc_extents, dev->id);
			put_device(dev);
			break;
		}

		rc = device_add(dev);
		if (rc) {
			xa_erase(&cxlr_dax->dc_extents, dev->id);
			put_device(dev);
			break;
		}
    ...
}

However, put_device() synchronously invokes dc_extent_release(), which
unconditionally executes a second xa_erase() for the same id:

drivers/cxl/core/extent.c:dc_extent_release() {
    ...
	group = dc_extent->group;
	cxled_release_extent(dc_extent->cxled, dc_extent);
	xa_erase(&group->cxlr_dax->dc_extents, dc_extent->dev.id);
    ...
}

Since the xarray is configured with XA_FLAGS_ALLOC1, a concurrent thread
processing another DCD event could allocate this id via xa_alloc() immediately
after the first xa_erase().

Could the second xa_erase() in the release function erroneously delete the
concurrent thread's newly allocated extent, breaking region capacity
accounting?


[Severity: High]
This is a pre-existing issue, but does the error handling in
cxl_realize_group() allow partial tag groups to be onlined?

If cxl_add_extent() fails for any extent in a More-chain group (e.g., due to an
overlap or memory allocation failure), the loop calls delete_extent_node() and
executes continue:

drivers/cxl/core/mbox.c:cxl_realize_group() {
    ...
		if (cxl_add_extent(mds, pos->extent, seq_num, existing) < 0) {
			dev_dbg(dev,
				"Tag %pUb: failed to add extent DPA:%#llx LEN:%#llx\n",
				tag,
				le64_to_cpu(pos->extent->start_dpa),
				le64_to_cpu(pos->extent->length));
			delete_extent_node(pos);
			continue;
		}
		group_cnt++;
    ...
}

By using continue, it bypasses the failure and leaves the remaining valid
extents in the group, resulting in a partial tag group being successfully
onlined.

Could this cause the host to surface a broken or incomplete DAX resource,
violating the architectural intent to either accept or reject tagged
allocations atomically?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625112638.550691-1-anisa.su@samsung.com?part=20