Re: [PATCH 4/7] sysfs: Implement sysfs_rename_link

Serge E. Hallyn
2010-01-12T17:31:04Z

Quoting Eric W. Biederman (ebiederm@xmission.com):
> From: Eric W. Biederman <ebiederm@xmission.com>
>
> Because of rename ordering problems we occassionally give false
> warnings about invalid sysfs operations. So using sysfs_rename
> create a sysfs_rename_link function that doesn't need strange
> workarounds.

It looks nice and clean esp compared to the code you're replacing.
OTOH I do wonder whether your sysfs_type(sd) != SYSFS_KOBJ_LINK check
will catch unexpected ways that device_rename() was being called.
Shouldn't, I realize, since sysfs_create_link() always sets SYSFS_KOBJ_LINK,
but the check wasn't there before, so stranger things have happened.

> Cc: Benjamin Thery <benjamin.thery@bull.net>
> Cc: Daniel Lezcano <dlezcano@fr.ibm.com>
> Cc: Tejun Heo <tj@kernel.org>
> Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>

Acked-by: Serge Hallyn <serue@us.ibm.com>

> ---
> fs/sysfs/symlink.c | 38 ++++++++++++++++++++++++++++++++++++++
> include/linux/sysfs.h | 9 +++++++++
> 2 files changed, 47 insertions(+), 0 deletions(-)
>
> diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
> index c5eff49..1b9a3a1 100644
> --- a/fs/sysfs/symlink.c
> +++ b/fs/sysfs/symlink.c
> @@ -123,6 +123,44 @@ void sysfs_remove_link(struct kobject * kobj, const char * name)
> sysfs_hash_and_remove(parent_sd, name);
> }
>
> +/**
> + * sysfs_rename_link - rename symlink in object's directory.
> + * @kobj: object we're acting for.
> + * @targ: object we're pointing to.
> + * @old: previous name of the symlink.
> + * @new: new name of the symlink.
> + *
> + * A helper function for the common rename symlink idiom.
> + */
> +int sysfs_rename_link(struct kobject *kobj, struct kobject *targ,
> + const char *old, const char *new)
> +{
> + struct sysfs_dirent *parent_sd, *sd = NULL;
> + int result;
> +
> + if (!kobj)
> + parent_sd = &sysfs_root;
> + else
> + parent_sd = kobj->sd;
> +
> + result = -ENOENT;
> + sd = sysfs_get_dirent(parent_sd, old);
> + if (!sd)
> + goto out;
> +
> + result = -EINVAL;
> + if (sysfs_type(sd) != SYSFS_KOBJ_LINK)
> + goto out;
> + if (sd->s_symlink.target_sd->s_dir.kobj != targ)
> + goto out;
> +
> + result = sysfs_rename(sd, parent_sd, new);
> +
> +out:
> + sysfs_put(sd);
> + return result;
> +}
> +
> static int sysfs_get_target_path(struct sysfs_dirent *parent_sd,
> struct sysfs_dirent *target_sd, char *path)
> {
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index cfa8308..e322573 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -110,6 +110,9 @@ int __must_check sysfs_create_link_nowarn(struct kobject *kobj,
> const char *name);
> void sysfs_remove_link(struct kobject *kobj, const char *name);
>
> +int sysfs_rename_link(struct kobject *kobj, struct kobject *target,
> + const char *old_name, const char *new_name);
> +
> int __must_check sysfs_create_group(struct kobject *kobj,
> const struct attribute_group *grp);
> int sysfs_update_group(struct kobject *kobj,
> @@ -203,6 +206,12 @@ static inline void sysfs_remove_link(struct kobject *kobj, const char *name)
> {
> }
>
> +static inline int sysfs_rename_link(struct kobject *k, struct kobject *t,
> + const char *old_name, const char *new_name)
> +{
> + return 0;
> +}
> +
> static inline int sysfs_create_group(struct kobject *kobj,
> const struct attribute_group *grp)
> {
> --
> 1.6.5.2.143.g8cc62