2008-06-10 09:09:39

by Cornelia Huck

[permalink] [raw]
Subject: Re: [PATCH] driver core: Suppress sysfs warnings for device_rename().

On Wed, 21 May 2008 10:05:56 +0200,
Cornelia Huck <[email protected]> wrote:

Just digging through my backlog: Is there any further interest in this
patch?

> On Tue, 20 May 2008 15:52:44 -0700,
> Greg KH <[email protected]> wrote:
>
> > On Tue, May 20, 2008 at 12:59:13PM +0200, Cornelia Huck wrote:
> > > OK, here is an actually-compiled patch with proper description and
> > > s-o-b. Comments?
> >
> > Looks good to me, feel free to add an:
> > Acked-by: Greg Kroah-Hartman <[email protected]>
> >
> > to it.
> >
> > David, are you going to take this through your tree?
>
> Here it is again, respun against today's git:
>
>
> driver core: Suppress sysfs warnings for device_rename().
>
> Renaming network devices to an already existing name is not
> something we want sysfs to print a scary warning for, since the
> callers can deal with this correctly. So let's introduce
> sysfs_create_link_nowarn() which gets rid of the common warning.
>
> Acked-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Cornelia Huck <[email protected]>
>
> ---
> drivers/base/core.c | 9 +++++----
> fs/sysfs/dir.c | 37 +++++++++++++++++++++++++++++++++++--
> fs/sysfs/symlink.c | 41 +++++++++++++++++++++++++++++++++--------
> fs/sysfs/sysfs.h | 1 +
> include/linux/sysfs.h | 10 ++++++++++
> 5 files changed, 84 insertions(+), 14 deletions(-)
>
> --- linux-2.6.orig/drivers/base/core.c
> +++ linux-2.6/drivers/base/core.c
> @@ -1282,8 +1282,9 @@ int device_rename(struct device *dev, ch
> if (old_class_name) {
> new_class_name = make_class_name(dev->class->name, &dev->kobj);
> if (new_class_name) {
> - error = sysfs_create_link(&dev->parent->kobj,
> - &dev->kobj, new_class_name);
> + error = sysfs_create_link_nowarn(&dev->parent->kobj,
> + &dev->kobj,
> + new_class_name);
> if (error)
> goto out;
> sysfs_remove_link(&dev->parent->kobj, old_class_name);
> @@ -1291,8 +1292,8 @@ int device_rename(struct device *dev, ch
> }
> #else
> if (dev->class) {
> - error = sysfs_create_link(&dev->class->subsys.kobj, &dev->kobj,
> - dev->bus_id);
> + error = sysfs_create_link_nowarn(&dev->class->subsys.kobj,
> + &dev->kobj, dev->bus_id);
> if (error)
> goto out;
> sysfs_remove_link(&dev->class->subsys.kobj, old_device_name);
> --- linux-2.6.orig/fs/sysfs/dir.c
> +++ linux-2.6/fs/sysfs/dir.c
> @@ -398,7 +398,7 @@ void sysfs_addrm_start(struct sysfs_addr
> }
>
> /**
> - * sysfs_add_one - add sysfs_dirent to parent
> + * __sysfs_add_one - add sysfs_dirent to parent without warning
> * @acxt: addrm context to use
> * @sd: sysfs_dirent to be added
> *
> @@ -417,7 +417,7 @@ void sysfs_addrm_start(struct sysfs_addr
> * 0 on success, -EEXIST if entry with the given name already
> * exists.
> */
> -int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
> +int __sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
> {
> if (sysfs_find_dirent(acxt->parent_sd, sd->s_name))
> return -EEXIST;
> @@ -435,6 +435,39 @@ int sysfs_add_one(struct sysfs_addrm_cxt
> }
>
> /**
> + * sysfs_add_one - add sysfs_dirent to parent
> + * @acxt: addrm context to use
> + * @sd: sysfs_dirent to be added
> + *
> + * Get @acxt->parent_sd and set sd->s_parent to it and increment
> + * nlink of parent inode if @sd is a directory and link into the
> + * children list of the parent.
> + *
> + * This function should be called between calls to
> + * sysfs_addrm_start() and sysfs_addrm_finish() and should be
> + * passed the same @acxt as passed to sysfs_addrm_start().
> + *
> + * LOCKING:
> + * Determined by sysfs_addrm_start().
> + *
> + * RETURNS:
> + * 0 on success, -EEXIST if entry with the given name already
> + * exists.
> + */
> +int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
> +{
> + int ret;
> +
> + ret = __sysfs_add_one(acxt, sd);
> + if (ret == -EEXIST) {
> + printk(KERN_WARNING "sysfs: duplicate filename '%s' "
> + "can not be created\n", sd->s_name);
> + WARN_ON(1);
> + }
> + return ret;
> +}
> +
> +/**
> * sysfs_remove_one - remove sysfs_dirent from parent
> * @acxt: addrm context to use
> * @sd: sysfs_dirent to be removed
> --- linux-2.6.orig/fs/sysfs/symlink.c
> +++ linux-2.6/fs/sysfs/symlink.c
> @@ -19,13 +19,8 @@
>
> #include "sysfs.h"
>
> -/**
> - * sysfs_create_link - create symlink between two objects.
> - * @kobj: object whose directory we're creating the link in.
> - * @target: object we're pointing to.
> - * @name: name of the symlink.
> - */
> -int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char * name)
> +static int sysfs_do_create_link(struct kobject *kobj, struct kobject *target,
> + const char *name, int warn)
> {
> struct sysfs_dirent *parent_sd = NULL;
> struct sysfs_dirent *target_sd = NULL;
> @@ -65,7 +60,10 @@ int sysfs_create_link(struct kobject * k
> target_sd = NULL; /* reference is now owned by the symlink */
>
> sysfs_addrm_start(&acxt, parent_sd);
> - error = sysfs_add_one(&acxt, sd);
> + if (warn)
> + error = sysfs_add_one(&acxt, sd);
> + else
> + error = __sysfs_add_one(&acxt, sd);
> sysfs_addrm_finish(&acxt);
>
> if (error)
> @@ -80,6 +78,33 @@ int sysfs_create_link(struct kobject * k
> }
>
> /**
> + * sysfs_create_link - create symlink between two objects.
> + * @kobj: object whose directory we're creating the link in.
> + * @target: object we're pointing to.
> + * @name: name of the symlink.
> + */
> +int sysfs_create_link(struct kobject *kobj, struct kobject *target,
> + const char *name)
> +{
> + return sysfs_do_create_link(kobj, target, name, 1);
> +}
> +
> +/**
> + * sysfs_create_link_nowarn - create symlink between two objects.
> + * @kobj: object whose directory we're creating the link in.
> + * @target: object we're pointing to.
> + * @name: name of the symlink.
> + *
> + * This function does the same as sysf_create_link(), but it
> + * doesn't warn if the link already exists.
> + */
> +int sysfs_create_link_nowarn(struct kobject *kobj, struct kobject *target,
> + const char *name)
> +{
> + return sysfs_do_create_link(kobj, target, name, 0);
> +}
> +
> +/**
> * sysfs_remove_link - remove symlink in object's directory.
> * @kobj: object we're acting for.
> * @name: name of the symlink to remove.
> --- linux-2.6.orig/fs/sysfs/sysfs.h
> +++ linux-2.6/fs/sysfs/sysfs.h
> @@ -107,6 +107,7 @@ struct sysfs_dirent *sysfs_get_active_tw
> void sysfs_put_active_two(struct sysfs_dirent *sd);
> void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt,
> struct sysfs_dirent *parent_sd);
> +int __sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd);
> int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd);
> void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd);
> void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt);
> --- linux-2.6.orig/include/linux/sysfs.h
> +++ linux-2.6/include/linux/sysfs.h
> @@ -101,6 +101,9 @@ void sysfs_remove_bin_file(struct kobjec
>
> int __must_check sysfs_create_link(struct kobject *kobj, struct kobject *target,
> const char *name);
> +int __must_check sysfs_create_link_nowarn(struct kobject *kobj,
> + struct kobject *target,
> + const char *name);
> void sysfs_remove_link(struct kobject *kobj, const char *name);
>
> int __must_check sysfs_create_group(struct kobject *kobj,
> @@ -180,6 +183,13 @@ static inline int sysfs_create_link(stru
> return 0;
> }
>
> +static inline int sysfs_create_link_nowarn(struct kobject *kobj,
> + struct kobject *target,
> + const char *name)
> +{
> + return 0;
> +}
> +
> static inline void sysfs_remove_link(struct kobject *kobj, const char *name)
> {
> }


2008-06-10 15:30:30

by Stephen Hemminger

[permalink] [raw]
Subject: Re: [PATCH] driver core: Suppress sysfs warnings for device_rename().

On Tue, 10 Jun 2008 11:09:08 +0200
Cornelia Huck <[email protected]> wrote:

> On Wed, 21 May 2008 10:05:56 +0200,
> Cornelia Huck <[email protected]> wrote:
>
> Just digging through my backlog: Is there any further interest in this
> patch?
>
> > On Tue, 20 May 2008 15:52:44 -0700,
> > Greg KH <[email protected]> wrote:
> >
> > > On Tue, May 20, 2008 at 12:59:13PM +0200, Cornelia Huck wrote:
> > > > OK, here is an actually-compiled patch with proper description and
> > > > s-o-b. Comments?
> > >
> > > Looks good to me, feel free to add an:
> > > Acked-by: Greg Kroah-Hartman <[email protected]>
> > >
> > > to it.
> > >
> > > David, are you going to take this through your tree?
> >
> > Here it is again, respun against today's git:
> >
> >
> > driver core: Suppress sysfs warnings for device_rename().
> >
> > Renaming network devices to an already existing name is not
> > something we want sysfs to print a scary warning for, since the
> > callers can deal with this correctly. So let's introduce
> > sysfs_create_link_nowarn() which gets rid of the common warning.
> >
> > Acked-by: Greg Kroah-Hartman <[email protected]>
> > Signed-off-by: Cornelia Huck <[email protected]>
> >

I you want to put the warnings back this is a reasonable way.