Eric W. Biederman wrote:
> From: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
>
> Modify sysfs to properly remove directories containing attributes and
> subdirectories. The code is relatively simple and means we don't have
> to worry about what might use this logic.
>
> In a quick survey I have only found /sys/dev/char and /sys/dev/block that are
> removing non-enmpty directories today (and they are exclusively filled with symlinks).
> So only removing empty directories does not appear to be an option.
>
> I don't hold sysfs_mutex across the entire operation as that is unneeded
> for coherence at the sysfs level and some level of coordination is expected
> at the upper layers.
>
> Signed-off-by: Eric W. Biederman <ebiederm@xxxxxxxxxxxxxxxxxx>
...
> -void sysfs_remove_subdir(struct sysfs_dirent *sd)
> -{
> - remove_dir(sd);
> + struct sysfs_dirent *sd = dir_sd;
> + mutex_lock(&sysfs_mutex);
> + while ((sysfs_type(sd) == SYSFS_DIR) && sd->s_dir.children)
> + sd = sd->s_dir.children;
> + if (sd != dir_sd)
> + sysfs_get(sd);
> + else
> + sd = NULL;
> + mutex_unlock(&sysfs_mutex);
> + return sd;
> }
Some blank lines wouldn't hurt, especially after local variable
declaration.
> -static void __sysfs_remove_dir(struct sysfs_dirent *dir_sd)
> +static void remove_dir(struct sysfs_dirent *dir_sd)
> {
> struct sysfs_addrm_cxt acxt;
> - struct sysfs_dirent **pos;
> -
> - if (!dir_sd)
> - return;
> + struct sysfs_dirent *sd;
>
> pr_debug("sysfs %s: removing dir\n", dir_sd->s_name);
> - sysfs_addrm_start(&acxt, dir_sd);
> - pos = &dir_sd->s_dir.children;
> - while (*pos) {
> - struct sysfs_dirent *sd = *pos;
>
> - if (sysfs_type(sd) != SYSFS_DIR)
> - sysfs_remove_one(&acxt, sd);
> - else
> - pos = &(*pos)->s_sibling;
> + while ((sd = sysfs_get_one(dir_sd))) {
> + sysfs_addrm_start(&acxt, sd->s_parent);
> + sysfs_remove_one(&acxt, sd);
> + sysfs_addrm_finish(&acxt);
> + sysfs_put(sd);
> }
> + sysfs_addrm_start(&acxt, dir_sd->s_parent);
> + sysfs_remove_one(&acxt, dir_sd);
> sysfs_addrm_finish(&acxt);
> +}
I agree we should be heading this way but what happens to attributes
or directories living below the subdirectories? If it's gonna handle
recursive case, I think it better do it properly. I had patches of
similar effect.
http://article.gmane.org/gmane.linux.kernel/582151
http://article.gmane.org/gmane.linux.kernel/582155
The patchset didn't really go anywhere but the recursive atomic
removal should be usable.
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
