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

[Benjamin Marzinski <bmarzins@redhat.com>]

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.

> +
> +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().

-Ben