| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1243628376-22905-17-git-send-email-ebiederm@xmission.com>
Date: Fri, 29 May 2009 13:19:27 -0700
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: Andrew Morton <akpm@...ux-foundation.org>,
Greg Kroah-Hartman <gregkh@...e.de>
Cc: <linux-kernel@...r.kernel.org>, Tejun Heo <tj@...nel.org>,
Cornelia Huck <cornelia.huck@...ibm.com>,
<linux-fsdevel@...r.kernel.org>,
Kay Sievers <kay.sievers@...y.org>, Greg KH <greg@...ah.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
"Eric W. Biederman" <ebiederm@...stanetworks.com>
Subject: [PATCH 17/26] sysfs: Kill sysfs_addrm_start and sysfs_addrm_finish
From: Eric W. Biederman <ebiederm@...ssion.com>
With lazy inode updates and dentry operations bringing everything
into sync on demand there is no longer any need to immediately
update the vfs or grab i_mutex to protect those updates as we
make changes to sysfs.
So stop updating the vfs inodes and move what remains of
sysfs_addrm_start and sysfs_addrm_finsih (just barely more than taking
the sysfs_mutex) into sysfs_add_one and sysfs_remove_one.
Acked-by: Tejun Heo <tj@...nel.org>
Signed-off-by: Eric W. Biederman <ebiederm@...stanetworks.com>
---
fs/sysfs/dir.c | 194 +++++++---------------------------------------------
fs/sysfs/file.c | 6 +--
fs/sysfs/inode.c | 16 ++---
fs/sysfs/symlink.c | 6 +--
fs/sysfs/sysfs.h | 17 +----
5 files changed, 34 insertions(+), 205 deletions(-)
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index b75c938..3caf9c6 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -382,62 +382,6 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type)
return NULL;
}
-static int sysfs_ilookup_test(struct inode *inode, void *arg)
-{
- struct sysfs_dirent *sd = arg;
- return inode->i_ino == sd->s_ino;
-}
-
-/**
- * sysfs_addrm_start - prepare for sysfs_dirent add/remove
- * @acxt: pointer to sysfs_addrm_cxt to be used
- * @parent_sd: parent sysfs_dirent
- *
- * 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.
- *
- * LOCKING:
- * Kernel thread context (may sleep). sysfs_mutex is locked on
- * return. i_mutex of parent inode is locked on return if
- * available.
- */
-void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt,
- struct sysfs_dirent *parent_sd)
-{
- struct inode *inode;
-
- memset(acxt, 0, sizeof(*acxt));
- acxt->parent_sd = parent_sd;
-
- /* Lookup parent inode. inode initialization is protected by
- * sysfs_mutex, so inode existence can be determined by
- * looking up inode while holding sysfs_mutex.
- */
- mutex_lock(&sysfs_mutex);
-
- inode = ilookup5(sysfs_sb, parent_sd->s_ino, sysfs_ilookup_test,
- parent_sd);
- if (inode) {
- WARN_ON(inode->i_state & I_NEW);
-
- /* parent inode available */
- acxt->parent_inode = inode;
-
- /* sysfs_mutex is below i_mutex in lock hierarchy.
- * First, trylock i_mutex. If fails, unlock
- * sysfs_mutex and lock them in order.
- */
- if (!mutex_trylock(&inode->i_mutex)) {
- mutex_unlock(&sysfs_mutex);
- mutex_lock(&inode->i_mutex);
- mutex_lock(&sysfs_mutex);
- }
- }
-}
-
/**
* sysfs_pathname - return full path to sysfs dirent
* @sd: sysfs_dirent whose path we want
@@ -460,161 +404,83 @@ static char *sysfs_pathname(struct sysfs_dirent *sd, char *path)
/**
* sysfs_add_one - add sysfs_dirent to parent
- * @acxt: addrm context to use
+ * @parent_sd: directory to add @sd into
* @sd: sysfs_dirent to be added
*
- * Get @acxt->parent_sd and set sd->s_parent to it and increment
+ * Get @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().
+ * Kernel thread context (may sleep). Grabs sysfs_mutex.
*
* 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 sysfs_add_one(struct sysfs_dirent *parent_sd, struct sysfs_dirent *sd)
{
struct iattr *ps_iattr;
- if (sysfs_find_dirent(acxt->parent_sd, sd->s_name)) {
- char *path = kzalloc(PATH_MAX, GFP_KERNEL);
+ mutex_lock(&sysfs_mutex);
+ if (sysfs_find_dirent(parent_sd, sd->s_name)) {
+ char *path;
+ mutex_unlock(&sysfs_mutex);
+
+ path = kzalloc(PATH_MAX, GFP_KERNEL);
WARN(1, KERN_WARNING
"sysfs: cannot create duplicate filename '%s'\n",
(path == NULL) ? sd->s_name :
- strcat(strcat(sysfs_pathname(acxt->parent_sd, path), "/"),
+ strcat(strcat(sysfs_pathname(parent_sd, path), "/"),
sd->s_name));
kfree(path);
return -EEXIST;
}
- sd->s_parent = sysfs_get(acxt->parent_sd);
-
- if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode)
- inc_nlink(acxt->parent_inode);
-
- acxt->cnt++;
-
+ sd->s_parent = sysfs_get(parent_sd);
sysfs_link_sibling(sd);
/* Update timestamps on the parent */
- ps_iattr = acxt->parent_sd->s_iattr;
+ ps_iattr = parent_sd->s_iattr;
if (ps_iattr)
ps_iattr->ia_ctime = ps_iattr->ia_mtime = CURRENT_TIME;
+ mutex_unlock(&sysfs_mutex);
return 0;
}
/**
* sysfs_remove_one - remove sysfs_dirent from parent
- * @acxt: addrm context to use
* @sd: sysfs_dirent to be removed
*
* Mark @sd removed and drop nlink of parent inode if @sd is a
* directory. @sd is unlinked from the children list.
*
- * 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().
+ * Kernel thread context (may sleep). Grabs sysfs_mutex.
*/
-void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
+void sysfs_remove_one(struct sysfs_dirent *sd)
{
struct iattr *ps_iattr;
BUG_ON(sd->s_flags & SYSFS_FLAG_REMOVED);
+ mutex_lock(&sysfs_mutex);
+
sysfs_unlink_sibling(sd);
/* Update timestamps on the parent */
- ps_iattr = acxt->parent_sd->s_iattr;
+ ps_iattr = sd->s_parent->s_iattr;
if (ps_iattr)
ps_iattr->ia_ctime = ps_iattr->ia_mtime = CURRENT_TIME;
sd->s_flags |= SYSFS_FLAG_REMOVED;
- sd->s_sibling = acxt->removed;
- acxt->removed = sd;
-
- if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode)
- drop_nlink(acxt->parent_inode);
- acxt->cnt++;
-}
-
-/**
- * sysfs_dec_nlink - Decrement link count for the specified sysfs_dirent
- * @sd: target sysfs_dirent
- *
- * Decrement nlink for @sd. @sd must have been unlinked from its
- * parent on entry to this function such that it can't be looked
- * up anymore.
- */
-static void sysfs_dec_nlink(struct sysfs_dirent *sd)
-{
- struct inode *inode;
-
- inode = ilookup(sysfs_sb, sd->s_ino);
- if (!inode)
- return;
-
- /* adjust nlink and update timestamp */
- mutex_lock(&inode->i_mutex);
-
- inode->i_ctime = CURRENT_TIME;
- drop_nlink(inode);
- if (sysfs_type(sd) == SYSFS_DIR)
- drop_nlink(inode);
-
- mutex_unlock(&inode->i_mutex);
-
- iput(inode);
-}
-
-/**
- * sysfs_addrm_finish - finish up sysfs_dirent add/remove
- * @acxt: addrm context to finish up
- *
- * Finish up sysfs_dirent add/remove. Resources acquired by
- * sysfs_addrm_start() are released and removed sysfs_dirents are
- * cleaned up. Timestamps on the parent inode are updated.
- *
- * LOCKING:
- * All mutexes acquired by sysfs_addrm_start() are released.
- */
-void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt)
-{
- /* release resources acquired by sysfs_addrm_start() */
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) {
- struct sysfs_dirent *sd = acxt->removed;
-
- acxt->removed = sd->s_sibling;
- sd->s_sibling = NULL;
-
- sysfs_dec_nlink(sd);
- sysfs_deactivate(sd);
- unmap_bin_file(sd);
- sysfs_put(sd);
- }
+ sysfs_deactivate(sd);
+ unmap_bin_file(sd);
+ sysfs_put(sd);
}
/**
@@ -673,7 +539,6 @@ static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd,
const char *name, struct sysfs_dirent **p_sd)
{
umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO;
- struct sysfs_addrm_cxt acxt;
struct sysfs_dirent *sd;
int rc;
@@ -684,10 +549,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_finish(&acxt);
+ rc = sysfs_add_one(parent_sd, sd);
if (rc == 0)
*p_sd = sd;
else
@@ -769,8 +632,6 @@ const struct inode_operations sysfs_dir_inode_operations = {
static void remove_dir(struct sysfs_dirent *dir_sd)
{
- struct sysfs_addrm_cxt acxt;
-
pr_debug("sysfs %s: removing dir\n", dir_sd->s_name);
/* Removing non-empty directories is not valid complain! */
@@ -787,9 +648,7 @@ static void remove_dir(struct sysfs_dirent *dir_sd)
mutex_unlock(&sysfs_mutex);
}
- sysfs_addrm_start(&acxt, dir_sd->s_parent);
- sysfs_remove_one(&acxt, dir_sd);
- sysfs_addrm_finish(&acxt);
+ sysfs_remove_one(dir_sd);
}
void sysfs_remove_subdir(struct sysfs_dirent *sd)
@@ -818,14 +677,11 @@ static struct sysfs_dirent *get_dirent_to_remove(struct sysfs_dirent *dir_sd)
static void __sysfs_remove_dir(struct sysfs_dirent *dir_sd)
{
- struct sysfs_addrm_cxt acxt;
struct sysfs_dirent *sd;
/* 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_remove_one(sd);
sysfs_put(sd);
}
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 31cfe1d..b512ce6 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -499,7 +499,6 @@ int sysfs_add_file_mode(struct sysfs_dirent *dir_sd,
const struct attribute *attr, int type, mode_t amode)
{
umode_t mode = (amode & S_IALLUGO) | S_IFREG;
- struct sysfs_addrm_cxt acxt;
struct sysfs_dirent *sd;
int rc;
@@ -508,10 +507,7 @@ int sysfs_add_file_mode(struct sysfs_dirent *dir_sd,
return -ENOMEM;
sd->s_attr.attr = (void *)attr;
- sysfs_addrm_start(&acxt, dir_sd);
- rc = sysfs_add_one(&acxt, sd);
- sysfs_addrm_finish(&acxt);
-
+ rc = sysfs_add_one(dir_sd, sd);
if (rc)
sysfs_put(sd);
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 1b7ed3c..ad9a30d 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -263,23 +263,17 @@ void sysfs_delete_inode(struct inode *inode)
int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name)
{
- struct sysfs_addrm_cxt acxt;
struct sysfs_dirent *sd;
if (!dir_sd)
return -ENOENT;
- sysfs_addrm_start(&acxt, dir_sd);
-
- sd = sysfs_find_dirent(dir_sd, name);
- if (sd)
- sysfs_remove_one(&acxt, sd);
-
- sysfs_addrm_finish(&acxt);
-
- if (sd)
+ sd = sysfs_get_dirent(dir_sd, name);
+ if (sd) {
+ sysfs_remove_one(sd);
+ sysfs_put(sd);
return 0;
- else
+ } else
return -ENOENT;
}
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 05e4984..fc5fc86 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -31,7 +31,6 @@ int sysfs_create_link(struct kobject *kobj, struct kobject *target,
struct sysfs_dirent *parent_sd = NULL;
struct sysfs_dirent *target_sd = NULL;
struct sysfs_dirent *sd = NULL;
- struct sysfs_addrm_cxt acxt;
int error;
BUG_ON(!name);
@@ -65,10 +64,7 @@ int sysfs_create_link(struct kobject *kobj, struct kobject *target,
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_finish(&acxt);
-
+ error = sysfs_add_one(parent_sd, sd);
if (error)
goto out_put;
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index f5b53cf..f17ebb8 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -77,16 +77,6 @@ static inline unsigned int sysfs_type(struct sysfs_dirent *sd)
}
/*
- * Context structure to be used while adding/removing nodes.
- */
-struct sysfs_addrm_cxt {
- struct sysfs_dirent *parent_sd;
- struct inode *parent_inode;
- struct sysfs_dirent *removed;
- int cnt;
-};
-
-/*
* mount.c
*/
extern struct sysfs_dirent sysfs_root;
@@ -106,11 +96,8 @@ extern const struct inode_operations sysfs_dir_inode_operations;
struct dentry *sysfs_get_dentry(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_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd);
-void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt);
+int sysfs_add_one(struct sysfs_dirent *parent_sd, struct sysfs_dirent *sd);
+void sysfs_remove_one(struct sysfs_dirent *sd);
struct sysfs_dirent *sysfs_find_dirent(struct sysfs_dirent *parent_sd,
const unsigned char *name);
--
1.6.3.1.54.g99dd.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists