[PATCH 04/24] sysfs: Normalize removing sysfs directories.

Eric W. Biederman
2009-05-28T23:03:40Z

From: Eric W. Biederman <ebiederm@xmission.com>

Individually wrap each call to sysfs_remove_one with sysfs_addrm_start
and sysfs_addrm_finish this prepares for these functions to be removed.

I don't change the logic of which dirents we remove. I do document
what we are doing and why what we are doing it.

A warning is added if we attempt to remove an non-empty directories
so that users that would prevsiosly have had leaks and other problems
like the scsi layer get a clear warning that there is a problem.

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@aristanetworks.com>
---
fs/sysfs/dir.c | 62 +++++++++++++++++++++++++++++++++++++++----------------
1 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index b95cc07..3e3a87f 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -732,12 +732,28 @@ const struct inode_operations sysfs_dir_inode_operations = {
.setattr = sysfs_setattr,
};

-static void remove_dir(struct sysfs_dirent *sd)
+static void remove_dir(struct sysfs_dirent *dir_sd)
{
struct sysfs_addrm_cxt acxt;

- sysfs_addrm_start(&acxt, sd->s_parent);
- sysfs_remove_one(&acxt, sd);
+ pr_debug("sysfs %s: removing dir\n", dir_sd->s_name);
+
+ /* Removing non-empty directories is not valid complain! */
+ if (unlikely(dir_sd->s_dir.children)) {
+ struct sysfs_dirent *sd;
+
+ WARN(1, KERN_WARNING "sysfs: removing non-empty dir: %s\n",
+ dir_sd->s_name);
+
+ mutex_lock(&sysfs_mutex);
+ for (sd = dir_sd->s_dir.children; sd; sd = sd->s_sibling)
+ printk(KERN_WARNING "%s/%s\n",
+ dir_sd->s_name, sd->s_name);
+ mutex_unlock(&sysfs_mutex);
+ }
+
+ sysfs_addrm_start(&acxt, dir_sd->s_parent);
+ sysfs_remove_one(&acxt, dir_sd);
sysfs_addrm_finish(&acxt);
}

@@ -746,27 +762,37 @@ void sysfs_remove_subdir(struct sysfs_dirent *sd)
remove_dir(sd);
}

+static struct sysfs_dirent *get_dirent_to_remove(struct sysfs_dirent *dir_sd)
+{
+ struct sysfs_dirent *sd;
+
+ mutex_lock(&sysfs_mutex);
+ for (sd = dir_sd->s_dir.children; sd; sd = sd->s_sibling) {
+ /* Directories might be owned by someone else
+ * making recursive directory removal unsafe.
+ */
+ if (sysfs_type(sd) == SYSFS_DIR)
+ continue;
+ break;
+ }
+ sysfs_get(sd);
+ mutex_unlock(&sysfs_mutex);
+
+ return sd;
+}

static void __sysfs_remove_dir(struct sysfs_dirent *dir_sd)
{
struct sysfs_addrm_cxt acxt;
- struct sysfs_dirent **pos;
-
- if (!dir_sd)
- return;
-
- 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;
+ struct sysfs_dirent *sd;

- if (sysfs_type(sd) != SYSFS_DIR)
- sysfs_remove_one(&acxt, sd);
- else
- pos = &(*pos)->s_sibling;
+ /* Remove children that we think are safe */
+ while ((sd = get_dirent_to_remove(dir_sd))) {
+ sysfs_addrm_start(&acxt, sd->s_parent);
+ sysfs_remove_one(&acxt, sd);
+ sysfs_addrm_finish(&acxt);
+ sysfs_put(sd);
}
- sysfs_addrm_finish(&acxt);

remove_dir(dir_sd);
}
--
1.6.3.1.54.g99dd.dirty