Re: [PATCH v11 05/31] cxl/mem: Expose dynamic ram 1 partition in sysfs

On Thu, Jun 25, 2026 at 06:12:42PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Empty sysfs directory created for absent dynamic RAM partitions.

Fixed.

> --
> 
> commit 839299dd9514d54314e661cd789de765da08d4f4
> Author: Ira Weiny <[email protected]>
> 
> cxl/mem: Expose dynamic ram 1 partition in sysfs
> 
> This commit exposes the details of the first dynamic ram partition via
> sysfs so user space can properly configure CXL regions.
> 
> > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> > --- a/drivers/cxl/core/memdev.c
> > +++ b/drivers/cxl/core/memdev.c
> 
> [ ... ]
> 
> > @@ -519,6 +551,29 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
> >  	.is_visible = cxl_pmem_visible,
> >  };
> >  
> > +static umode_t cxl_dynamic_ram_1_visible(struct kobject *kobj, struct attribute *a, int n)
> > +{
> > +	struct device *dev = kobj_to_dev(kobj);
> > +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> > +	struct cxl_dpa_perf *perf = part_perf(cxlmd->cxlds, CXL_PARTMODE_DYNAMIC_RAM_1);
> > +
> > +	if (a == &dev_attr_dynamic_ram_1_qos_class.attr &&
> > +	    (!perf || perf->qos_class == CXL_QOS_CLASS_INVALID))
> > +		return 0;
> > +
> > +	if (a == &dev_attr_dynamic_ram_1_size.attr &&
> > +	    (!cxl_part_size(cxlmd->cxlds, CXL_PARTMODE_DYNAMIC_RAM_1)))
> > +		return 0;
> 
> [Severity: Low]
> Should cxl_dynamic_ram_1_visible() return SYSFS_GROUP_INVISIBLE here when the
> dynamic RAM partition is absent?
> 
> Since cxl_memdev_dynamic_ram_1_attribute_group is a named group
> (.name = "dynamic_ram_1"), returning 0 hides the individual attributes but
> still creates the group directory.
> 
> Won't this result in a permanently empty "dynamic_ram_1" directory in sysfs if
> the size is 0, rather than suppressing the directory entirely?
> 
Fixed.

If a DC partition's size is 0, better not the expose the directory at
all, instead of hide both attributes.

Ram and pmem always expose the size attribute, even if the
size of the ram/pmem partition is 0, but that makes less sense for a DC
partition, especially if support for multiple partitions is added later.

> > +
> > +	return a->mode;
> > +}
> > +
> > +static struct attribute_group cxl_memdev_dynamic_ram_1_attribute_group = {
> > +	.name = "dynamic_ram_1",
> > +	.attrs = cxl_memdev_dynamic_ram_1_attributes,
> > +	.is_visible = cxl_dynamic_ram_1_visible,
> > +};
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5