Re: [PATCH 08/13] libmultipath: Add sysfs helpers

[John Garry <john.g.garry@oracle.com>]

On 27/02/2026 19:05, Benjamin Marzinski wrote:
> On Wed, Feb 25, 2026 at 03:32:20PM +0000, John Garry wrote:
>> Add helpers for driver sysfs code for the following functionality:
>> - get/set iopolicy with mpath_iopolicy_store() and mpath_iopolicy_show()
>> - show device path per NUMA node
>> - "multipath" attribute group, equivalent to nvme_ns_mpath_attr_group
>> - device groups attribute array, similar to nvme_ns_attr_groups but not
>>    containing NVMe members.
>>
>> Note that mpath_iopolicy_store() has a update callback to allow same
>> functionality as nvme_subsys_iopolicy_update() be run for clearing paths.
>>
>> Signed-off-by: John Garry <john.g.garry@oracle.com>
>>
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 1ce57b9b14d2e..c05b4d25ca223 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
>> @@ -745,6 +745,116 @@ void mpath_device_set_live(struct mpath_disk *mpath_disk,
>>   }
>>   EXPORT_SYMBOL_GPL(mpath_device_set_live);
>>   
>> +static struct attribute dummy_attr = {
>> +	.name = "dummy",
>> +};
>> +
>> +static struct attribute *mpath_attrs[] = {
>> +	&dummy_attr,
>> +	NULL
>> +};
>> +
>> +static bool multipath_sysfs_group_visible(struct kobject *kobj)
>> +{
>> +	struct device *dev = container_of(kobj, struct device, kobj);
>> +	struct gendisk *disk = dev_to_disk(dev);
>> +
>> +	return is_mpath_head(disk);
>> +}
>> +
>> +static bool multipath_sysfs_attr_visible(struct kobject *kobj,
>> +		struct attribute *attr, int n)
>> +{
>> +	return false;
>> +}
>> +
>> +DEFINE_SYSFS_GROUP_VISIBLE(multipath_sysfs)
> 
> nitpick: this could use DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE instead.
> 

Yes, that seems reasonable. And, FWIW, I think that 
multipath_sysfs_attr_visible() should return umode_t.

BTW, this is same as mainline NVMe code, so that could be updated first.

>> +
>> +const struct attribute_group mpath_attr_group = {
>> +	.name           = "multipath",
>> +	.attrs		= mpath_attrs,
>> +	.is_visible     = SYSFS_GROUP_VISIBLE(multipath_sysfs),
>> +};
>> +EXPORT_SYMBOL_GPL(mpath_attr_group);
>> +
>> +const struct attribute_group *mpath_device_groups[] = {
>> +	&mpath_attr_group,
>> +	NULL
>> +};
>> +EXPORT_SYMBOL_GPL(mpath_device_groups);
>> +
>> +ssize_t mpath_iopolicy_show(struct mpath_iopolicy *mpath_iopolicy, char *buf)
>> +{
>> +	return sysfs_emit(buf, "%s\n",
>> +		mpath_iopolicy_names[mpath_read_iopolicy(mpath_iopolicy)]);
>> +}
>> +EXPORT_SYMBOL_GPL(mpath_iopolicy_show);
>> +
>> +static void mpath_iopolicy_update(struct mpath_iopolicy *mpath_iopolicy,
>> +		int iopolicy, void (*update)(void *), void *data)
>> +{
>> +	int old_iopolicy = READ_ONCE(mpath_iopolicy->iopolicy);
>> +
>> +	if (old_iopolicy == iopolicy)
>> +		return;
>> +
>> +	WRITE_ONCE(mpath_iopolicy->iopolicy, iopolicy);
>> +
>> +	/*
>> +	 * iopolicy changes clear the mpath by design, which @update
>> +	 * must do.
>> +	 */
>> +	update(data);
>> +
>> +	pr_err("iopolicy changed from %s to %s\n",
>> +		mpath_iopolicy_names[old_iopolicy],
>> +		mpath_iopolicy_names[iopolicy]);
> 
> I not sure this warrants a pr_err().
> 

Agreed, I can downgrade this to something like pr_info or notice.

Thanks