[PATCH 06/22] sysfs: restructure addrm helpers

From: Tejun Heo
Date: Thu Sep 20 2007 - 04:08:22 EST

Next message: Tejun Heo: "[PATCH 08/22] sysfs: implement sysfs_dirent based directory interface"
Previous message: Tejun Heo: "[PATCH 02/22] sysfs: separate out sysfs-kobject.h and fs/sysfs/kobject.c"
In reply to: Tejun Heo: "[PATCH 02/22] sysfs: separate out sysfs-kobject.h and fs/sysfs/kobject.c"
Next in thread: Tejun Heo: "[PATCH 08/22] sysfs: implement sysfs_dirent based directory interface"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Restruct addrm helpers such that it can remove nodes under different
parents at one go.

sysfs_addrm_start() now doesn't take @sd. It now only initializes
@acxt and lock sysfs_mutex. Parent inode lookup now lives in
sysfs_addrm_get_parent_inode() and sysfs_add/remove_one() call them
directly. The function unlocks i_mutex of the previous inode and
grabs and locks the inode for the specified @parent_sd. This allows
sysfs_add/remove_one() to be called for any node.

This will be used to recursively remove sysfs nodes.

Signed-off-by: Tejun Heo <htejun@xxxxxxxxx>
---
fs/sysfs/dir.c | 144 +++++++++++++++++++++++++++++++++------------------
fs/sysfs/file.c | 4 +-
fs/sysfs/inode.c | 2 +-
fs/sysfs/symlink.c | 4 +-
fs/sysfs/sysfs.h | 10 ++--
5 files changed, 103 insertions(+), 61 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 7a6be9a..b1cf090 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -375,6 +375,27 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type)
return NULL;
}

+/**
+ * sysfs_addrm_start - prepare for sysfs_dirent add/remove
+ * @acxt: pointer to sysfs_addrm_cxt to be used
+ *
+ * This function is called when the caller is about to add or
+ * remove sysfs_dirents. This function initializes @acxt and
+ * acquires sysfs_mutex. @acxt is used to keep and pass context
+ * to other addrm functions.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep). sysfs_mutex is locked on
+ * return.
+ */
+void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt)
+{
+ memset(acxt, 0, sizeof(*acxt));
+ acxt->removed_tail = &acxt->removed;
+
+ mutex_lock(&sysfs_mutex);
+}
+
static int sysfs_ilookup_test(struct inode *inode, void *arg)
{
struct sysfs_dirent *sd = arg;
@@ -382,39 +403,53 @@ static int sysfs_ilookup_test(struct inode *inode, void *arg)
}

/**
- * sysfs_addrm_start - prepare for sysfs_dirent add/remove
- * @acxt: pointer to sysfs_addrm_cxt to be used
- * @parent_sd: parent sysfs_dirent
+ * sysfs_addrm_get_parent_inode - get parent inode for add/rm
+ * @acxt: addrm context to prepare parent inode for
+ * @new_parent: parent to prepare (can be NULL)
*
- * This function is called when the caller is about to add or
- * remove sysfs_dirent under @parent_sd. This function acquires
- * sysfs_mutex, grabs inode for @parent_sd if available and lock
- * i_mutex of it. @acxt is used to keep and pass context to
- * other addrm functions.
+ * Release inode of the current @acxt->parent_sd and prepare
+ * inode of @new_parent. If @new_parent is NULL, the current
+ * parent inode is released.
*
* LOCKING:
- * Kernel thread context (may sleep). sysfs_mutex is locked on
- * return. i_mutex of parent inode is locked on return if
- * available.
+ * mutex_lock(sysfs_mutex). Returns with i_mutex of @new_parent
+ * locked if available. sysfs_mutex might be released and
+ * regrabbed.
+ *
+ * RETURNS:
+ * inode of @new_parent if available, NULL otherwise.
*/
-void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt,
- struct sysfs_dirent *parent_sd)
+static struct inode *sysfs_addrm_get_parent_inode(struct sysfs_addrm_cxt *acxt,
+ struct sysfs_dirent *new_parent)
{
struct inode *inode;

- memset(acxt, 0, sizeof(*acxt));
- acxt->parent_sd = parent_sd;
- acxt->removed_tail = &acxt->removed;
+ /* nothing to do if new equals current */
+ if (new_parent == acxt->parent_sd)
+ goto out;
+
+ /* if we're holding inode of the current parent, release it */
+ if ((inode = acxt->parent_inode)) {
+ mutex_unlock(&inode->i_mutex);
+ iput(inode);
+ }
+
+ acxt->parent_sd = NULL;
+ acxt->parent_inode = NULL;
+
+ if (!new_parent)
+ goto out;
+
+ /* make @new_parent the current one */
+ acxt->parent_sd = new_parent;

/* Lookup parent inode. inode initialization and I_NEW
- * clearing are protected by sysfs_mutex. By grabbing it and
- * looking up with _nowait variant, inode state can be
+ * clearing are protected by sysfs_mutex. By looking up with
+ * _nowait variant while holding it, inode state can be
* determined reliably.
*/
- mutex_lock(&sysfs_mutex);
-
- inode = ilookup5_nowait(sysfs_sb, parent_sd->s_ino, sysfs_ilookup_test,
- parent_sd);
+ inode = ilookup5_nowait(sysfs_sb, new_parent->s_ino, sysfs_ilookup_test,
+ new_parent);

if (inode && !(inode->i_state & I_NEW)) {
/* parent inode available */
@@ -431,16 +466,19 @@ void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt,
}
} else
iput(inode);
+ out:
+ return acxt->parent_inode;
}

/**
* sysfs_add_one - add sysfs_dirent to parent
* @acxt: addrm context to use
+ * @parent: parent sysfs_dirent to add sysfs_dirent under
* @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.
+ * Get @parent 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
@@ -453,20 +491,26 @@ void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt,
* 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 *parent,
+ struct sysfs_dirent *sd)
{
- if (sysfs_find_dirent(acxt->parent_sd, sd->s_name))
- return -EEXIST;
+ struct inode *parent_inode;

- sd->s_parent = sysfs_get(acxt->parent_sd);
+ if (sysfs_find_dirent(parent, sd->s_name))
+ return -EEXIST;

- if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode)
- inc_nlink(acxt->parent_inode);
+ parent_inode = sysfs_addrm_get_parent_inode(acxt, parent);

- acxt->cnt++;
+ sd->s_parent = sysfs_get(parent);

sysfs_link_sibling(sd);

+ if (parent_inode) {
+ parent_inode->i_ctime = parent_inode->i_mtime = CURRENT_TIME;
+ if (sysfs_type(sd) == SYSFS_DIR)
+ inc_nlink(parent_inode);
+ }
+
return 0;
}

@@ -487,8 +531,12 @@ 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)
{
+ struct inode *parent_inode;
+
BUG_ON(sd->s_flags & SYSFS_FLAG_REMOVED);

+ parent_inode = sysfs_addrm_get_parent_inode(acxt, sd->s_parent);
+
sysfs_unlink_sibling(sd);

sd->s_flags |= SYSFS_FLAG_REMOVED;
@@ -497,10 +545,11 @@ void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
acxt->removed_tail = &sd->s_sibling;
*acxt->removed_tail = NULL;

- if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode)
- drop_nlink(acxt->parent_inode);
-
- acxt->cnt++;
+ if (parent_inode) {
+ parent_inode->i_ctime = parent_inode->i_mtime = CURRENT_TIME;
+ if (sysfs_type(sd) == SYSFS_DIR)
+ drop_nlink(parent_inode);
+ }
}

/**
@@ -568,18 +617,11 @@ repeat:
*/
void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt)
{
- /* release resources acquired by sysfs_addrm_start() */
+ /* Release resources acquired by
+ * sysfs_addrm_get_parent_inode() and sysfs_addrm_start().
+ */
+ sysfs_addrm_get_parent_inode(acxt, NULL);
mutex_unlock(&sysfs_mutex);
- if (acxt->parent_inode) {
- struct inode *inode = acxt->parent_inode;
-
- /* if added/removed, update timestamps on the parent */
- if (acxt->cnt)
- inode->i_ctime = inode->i_mtime = CURRENT_TIME;
-
- mutex_unlock(&inode->i_mutex);
- iput(inode);
- }

/* kill removed sysfs_dirents */
while (acxt->removed) {
@@ -695,8 +737,8 @@ static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd,
sd->s_dir.kobj = kobj;

/* link in */
- sysfs_addrm_start(&acxt, parent_sd);
- rc = sysfs_add_one(&acxt, sd);
+ sysfs_addrm_start(&acxt);
+ rc = sysfs_add_one(&acxt, parent_sd, sd);
sysfs_addrm_finish(&acxt);

if (rc == 0)
@@ -778,7 +820,7 @@ static void remove_dir(struct sysfs_dirent *sd)
{
struct sysfs_addrm_cxt acxt;

- sysfs_addrm_start(&acxt, sd->s_parent);
+ sysfs_addrm_start(&acxt);
sysfs_remove_one(&acxt, sd);
sysfs_addrm_finish(&acxt);
}
@@ -798,7 +840,7 @@ static void __sysfs_remove_dir(struct sysfs_dirent *dir_sd)
return;

pr_debug("sysfs %s: removing dir\n", dir_sd->s_name);
- sysfs_addrm_start(&acxt, dir_sd);
+ sysfs_addrm_start(&acxt);
pos = &dir_sd->s_dir.children;
while (*pos) {
struct sysfs_dirent *sd = *pos;
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 8d8e1ee..fb77d54 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -581,8 +581,8 @@ int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr,
return -ENOMEM;
sd->s_attr.attr = (void *)attr;

- sysfs_addrm_start(&acxt, dir_sd);
- rc = sysfs_add_one(&acxt, sd);
+ sysfs_addrm_start(&acxt);
+ rc = sysfs_add_one(&acxt, dir_sd, sd);
sysfs_addrm_finish(&acxt);

if (rc)
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 2210cf0..95bde29 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -214,7 +214,7 @@ int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name)
if (!dir_sd)
return -ENOENT;

- sysfs_addrm_start(&acxt, dir_sd);
+ sysfs_addrm_start(&acxt);

sd = sysfs_find_dirent(dir_sd, name);
if (sd)
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 982085c..897cc2f 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -90,8 +90,8 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char
sd->s_symlink.target_sd = target_sd;
target_sd = NULL; /* reference is now owned by the symlink */

- sysfs_addrm_start(&acxt, parent_sd);
- error = sysfs_add_one(&acxt, sd);
+ sysfs_addrm_start(&acxt);
+ error = sysfs_add_one(&acxt, parent_sd, sd);
sysfs_addrm_finish(&acxt);

if (error)
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index db1a433..1f3915f 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -66,13 +66,13 @@ static inline unsigned int sysfs_type(struct sysfs_dirent *sd)
}

/*
- * Context structure to be used while adding/removing nodes.
+ * Context structure to be used while adding/removing nodes. Don't
+ * dereference outside of addrm functions.
*/
struct sysfs_addrm_cxt {
struct sysfs_dirent *parent_sd;
struct inode *parent_inode;
struct sysfs_dirent *removed, **removed_tail;
- int cnt;
};

/*
@@ -97,9 +97,9 @@ struct sysfs_dirent *sysfs_get_active(struct sysfs_dirent *sd);
void sysfs_put_active(struct sysfs_dirent *sd);
struct sysfs_dirent *sysfs_get_active_two(struct sysfs_dirent *sd);
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);
+void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt);
+int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *parent,
+ 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);

--
1.5.0.3

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Tejun Heo: "[PATCH 08/22] sysfs: implement sysfs_dirent based directory interface"
Previous message: Tejun Heo: "[PATCH 02/22] sysfs: separate out sysfs-kobject.h and fs/sysfs/kobject.c"
In reply to: Tejun Heo: "[PATCH 02/22] sysfs: separate out sysfs-kobject.h and fs/sysfs/kobject.c"
Next in thread: Tejun Heo: "[PATCH 08/22] sysfs: implement sysfs_dirent based directory interface"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]