public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 5/6] sysfs: add support for binary attributes in groups
@ 2013-07-10 23:00 Guenter Roeck
  2013-07-10 23:59 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2013-07-10 23:00 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, oliver+list, khali

On Wed, Jul 10, 2013 at 01:05:13PM -0700, Greg Kroah-Hartman wrote:
> groups should be able to support binary attributes, just like it
> supports "normal" attributes.  This lets us only handle one type of
> structure, groups, throughout the driver core and subsystems, making
> binary attributes a "full fledged" part of the driver model, and not
> something just "tacked on".
> 
> Reported-by: Oliver Schinagl <oliver+list@schinagl.nl>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  fs/sysfs/group.c      | 18 ++++++++++++++++--
>  include/linux/sysfs.h |  4 ++--
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> index aec3d5c..d8d8a8f 100644
> --- a/fs/sysfs/group.c
> +++ b/fs/sysfs/group.c
> @@ -20,16 +20,19 @@ static void remove_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
>  			 const struct attribute_group *grp)
>  {
>  	struct attribute *const* attr;
> -	int i;
> +	struct bin_attribute *const* bin_attr;

checkpatch doesn't like the above and similar declarations below.

Yes, I know, nitpick, just saying ;).

>  
> -	for (i = 0, attr = grp->attrs; *attr; i++, attr++)
> +	for (attr = grp->attrs; *attr; attr++)
>  		sysfs_hash_and_remove(dir_sd, NULL, (*attr)->name);
> +	for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++)
> +		sysfs_remove_bin_file(kobj, *bin_attr);
>  }
>  
>  static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
>  			const struct attribute_group *grp, int update)
>  {
>  	struct attribute *const* attr;
> +	struct bin_attribute *const* bin_attr;
>  	int error = 0, i;
>  
>  	for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++) {
> @@ -52,6 +55,17 @@ static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
>  	}
>  	if (error)
>  		remove_files(dir_sd, kobj, grp);

You might want to abort here if there was an error.

> +
> +	for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {

What happens here if grp->bin_attrs is NULL ? Doesn't that cause a NULL
reference when accessing *bin_attr ?

[ The problem didn't exist before since attr was supposed to be non-NULL.
  It may now be a problem if only bin_attr is provided, ie if there are
  only binary attribute files.
  Or maybe I am just tired and there is no problem... ]

> +		if (update)
> +			sysfs_remove_bin_file(kobj, *bin_attr);
> +		error = sysfs_create_bin_file(kobj, *bin_attr);
> +		if (error)
> +			break;
> +	}
> +	if (error)
> +		remove_files(dir_sd, kobj, grp);
> +
>  	return error;
>  }
>  
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index d50a96b..2c3b6a3 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -21,6 +21,7 @@
>  
>  struct kobject;
>  struct module;
> +struct bin_attribute;
>  enum kobj_ns_type;
>  
>  struct attribute {
> @@ -59,10 +60,9 @@ struct attribute_group {
>  	umode_t			(*is_visible)(struct kobject *,
>  					      struct attribute *, int);
>  	struct attribute	**attrs;
> +	struct bin_attribute	**bin_attrs;
>  };
>  
> -
> -
>  /**
>   * Use these macros to make defining attributes easier. See include/linux/device.h
>   * for examples..
> -- 
> 1.8.3.rc0.20.gb99dd2e
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Driver core and sysfs changes for attribute groups
@ 2013-07-10 20:05 Greg Kroah-Hartman
  2013-07-10 20:05 ` [PATCH 5/6] sysfs: add support for binary attributes in groups Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2013-07-10 20:05 UTC (permalink / raw)
  To: linux-kernel; +Cc: oliver+list, linux, khali

Hi all,

Guenter and Oliver have been pointing out a few limitations of the
driver core's ability to create files properly (i.e. in a way that
doesn't race with userspace.)  The driver core allows this, but it
doesn't export that ability to drivers very easily, and for binary
files, not at all.

So here's a set of 6 patches that I'll be queueing up to go to Linus in
time for 3.11 so that people can start using them in their driver
subsystems.  It adds some new macros to make using attributes and
attribute groups easier, adds binary file capabilities to attribute
groups, and finally, lets subsystems (like platform drivers) set a
attribute group for when their device is created.

If anyone has any problems with these patches, please let me know.

Guenter, I've tweaked your original patch a bit, changing the name of
the function and putting the kernel doc comments in the correct place so
the build doesn't complain about it.

I also have a set of follow-on patches, about 50+ big so far, that goes
through the kernel and converts different drivers and subsystems to
properly use attribute groups, instead of open-coding binary files and
attributes.  Those patches will be sent out later, and will be for 3.12
as they aren't needed at the moment, this infrastructure changes are
needed first.

thanks,

greg k-h


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-07-11  0:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-10 23:00 [PATCH 5/6] sysfs: add support for binary attributes in groups Guenter Roeck
2013-07-10 23:59 ` Greg Kroah-Hartman
2013-07-11  0:15   ` Greg Kroah-Hartman
  -- strict thread matches above, loose matches on Subject: below --
2013-07-10 20:05 Driver core and sysfs changes for attribute groups Greg Kroah-Hartman
2013-07-10 20:05 ` [PATCH 5/6] sysfs: add support for binary attributes in groups Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox