Re: [PATCH v2 13/18] sysfs: Allow bin_attributes to be added to groups

[Alistair Francis <Alistair.Francis@wdc.com>]

On Sun, 2024-06-30 at 21:48 +0200, Lukas Wunner wrote:
> Commit dfa87c824a9a ("sysfs: allow attributes to be added to groups")
> introduced dynamic addition of sysfs attributes to groups.
> 
> Allow the same for bin_attributes, in support of a subsequent commit
> which adds various bin_attributes every time a PCI device is
> authenticated.
> 
> Addition of bin_attributes to groups differs from regular attributes
> in
> that different kernfs_ops are selected by
> sysfs_add_bin_file_mode_ns()
> vis-à-vis sysfs_add_file_mode_ns().
> 
> So call either of those two functions from sysfs_add_file_to_group()
> based on an additional boolean parameter and add two wrapper
> functions,
> one for bin_attributes and another for regular attributes.
> 
> Removal of bin_attributes from groups does not require a
> differentiation
> for bin_attributes and can use the same code path as regular
> attributes.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: Alan Stern <stern@rowland.harvard.edu>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  fs/sysfs/file.c       | 69 ++++++++++++++++++++++++++++++++++++-----
> --
>  include/linux/sysfs.h | 19 ++++++++++++
>  2 files changed, 78 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index d1995e2d6c94..9268232781b5 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -383,14 +383,14 @@ int sysfs_create_files(struct kobject *kobj,
> const struct attribute * const *ptr
>  }
>  EXPORT_SYMBOL_GPL(sysfs_create_files);
>  
> -/**
> - * sysfs_add_file_to_group - add an attribute file to a pre-existing
> group.
> - * @kobj: object we're acting for.
> - * @attr: attribute descriptor.
> - * @group: group name.
> - */
> -int sysfs_add_file_to_group(struct kobject *kobj,
> -		const struct attribute *attr, const char *group)
> +static const struct bin_attribute *to_bin_attr(const struct
> attribute *attr)
> +{
> +	return container_of(attr, struct bin_attribute, attr);
> +}
> +
> +static int __sysfs_add_file_to_group(struct kobject *kobj,
> +				     const struct attribute *attr,
> +				     const char *group, bool
> is_bin_attr)
>  {
>  	struct kernfs_node *parent;
>  	kuid_t uid;
> @@ -408,14 +408,49 @@ int sysfs_add_file_to_group(struct kobject
> *kobj,
>  		return -ENOENT;
>  
>  	kobject_get_ownership(kobj, &uid, &gid);
> -	error = sysfs_add_file_mode_ns(parent, attr, attr->mode,
> uid, gid,
> -				       NULL);
> +	if (is_bin_attr)
> +		error = sysfs_add_bin_file_mode_ns(parent,
> to_bin_attr(attr),
> +						   attr->mode, uid,
> gid, NULL);
> +	else
> +		error = sysfs_add_file_mode_ns(parent, attr,
> +					       attr->mode, uid, gid,
> NULL);
>  	kernfs_put(parent);
>  
>  	return error;
>  }
> +
> +/**
> + * sysfs_add_file_to_group - add an attribute file to a pre-existing
> group.
> + * @kobj: object we're acting for.
> + * @attr: attribute descriptor.
> + * @group: group name.
> + *
> + * Returns 0 on success or error code on failure.
> + */
> +int sysfs_add_file_to_group(struct kobject *kobj,
> +			    const struct attribute *attr,
> +			    const char *group)
> +{
> +	return __sysfs_add_file_to_group(kobj, attr, group, false);
> +}
>  EXPORT_SYMBOL_GPL(sysfs_add_file_to_group);
>  
> +/**
> + * sysfs_add_bin_file_to_group - add bin_attribute file to pre-
> existing group.
> + * @kobj: object we're acting for.
> + * @attr: attribute descriptor.
> + * @group: group name.
> + *
> + * Returns 0 on success or error code on failure.
> + */
> +int sysfs_add_bin_file_to_group(struct kobject *kobj,
> +				const struct bin_attribute *attr,
> +				const char *group)
> +{
> +	return __sysfs_add_file_to_group(kobj, &attr->attr, group,
> true);
> +}
> +EXPORT_SYMBOL_GPL(sysfs_add_bin_file_to_group);
> +
>  /**
>   * sysfs_chmod_file - update the modified mode value on an object
> attribute.
>   * @kobj: object we're acting for.
> @@ -565,6 +600,20 @@ void sysfs_remove_file_from_group(struct kobject
> *kobj,
>  }
>  EXPORT_SYMBOL_GPL(sysfs_remove_file_from_group);
>  
> +/**
> + * sysfs_remove_bin_file_from_group - remove bin_attribute file from
> group.
> + * @kobj: object we're acting for.
> + * @attr: attribute descriptor.
> + * @group: group name.
> + */
> +void sysfs_remove_bin_file_from_group(struct kobject *kobj,
> +				      const struct bin_attribute
> *attr,
> +				      const char *group)
> +{
> +	sysfs_remove_file_from_group(kobj, &attr->attr, group);
> +}
> +EXPORT_SYMBOL_GPL(sysfs_remove_bin_file_from_group);
> +
>  /**
>   *	sysfs_create_bin_file - create binary file for object.
>   *	@kobj:	object.
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index a7d725fbf739..aff1d81e8971 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -451,6 +451,12 @@ int sysfs_add_file_to_group(struct kobject
> *kobj,
>  			const struct attribute *attr, const char
> *group);
>  void sysfs_remove_file_from_group(struct kobject *kobj,
>  			const struct attribute *attr, const char
> *group);
> +int sysfs_add_bin_file_to_group(struct kobject *kobj,
> +				const struct bin_attribute *attr,
> +				const char *group);
> +void sysfs_remove_bin_file_from_group(struct kobject *kobj,
> +				      const struct bin_attribute
> *attr,
> +				      const char *group);
>  int sysfs_merge_group(struct kobject *kobj,
>  		       const struct attribute_group *grp);
>  void sysfs_unmerge_group(struct kobject *kobj,
> @@ -660,6 +666,19 @@ static inline void
> sysfs_remove_file_from_group(struct kobject *kobj,
>  {
>  }
>  
> +static inline int sysfs_add_bin_file_to_group(struct kobject *kobj,
> +					      const struct
> bin_attribute *attr,
> +					      const char *group)
> +{
> +	return 0;
> +}
> +
> +static inline void sysfs_remove_bin_file_from_group(struct kobject
> *kobj,
> +					      const struct
> bin_attribute *attr,
> +					      const char *group)
> +{
> +}
> +
>  static inline int sysfs_merge_group(struct kobject *kobj,
>  		       const struct attribute_group *grp)
>  {