Revert "kernfs: remove KERNFS_REMOVED"

This reverts commit ae34372eb8.

Tejun writes:
        I'm sorry but can you please revert the whole series?
        get_active() waiting while a node is deactivated has potential
        to lead to deadlock and that deactivate/reactivate interface is
        something fundamentally flawed and that cgroup will have to work
        with the remove_self() like everybody else.  IOW, I think the
        first posting was correct.

Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Greg Kroah-Hartman 2014-01-13 14:36:03 -08:00
parent 4f4b1b6471
commit 798c75a0d4
5 changed files with 44 additions and 61 deletions

View File

@ -127,7 +127,6 @@ static void kernfs_unlink_sibling(struct kernfs_node *kn)
kn->parent->dir.subdirs--; kn->parent->dir.subdirs--;
rb_erase(&kn->rb, &kn->parent->dir.children); rb_erase(&kn->rb, &kn->parent->dir.children);
RB_CLEAR_NODE(&kn->rb);
} }
/** /**
@ -178,16 +177,18 @@ void kernfs_put_active(struct kernfs_node *kn)
} }
/** /**
* kernfs_drain - drain kernfs_node * kernfs_deactivate - deactivate kernfs_node
* @kn: kernfs_node to drain * @kn: kernfs_node to deactivate
* *
* Drain existing usages. * Deny new active references and drain existing ones.
*/ */
static void kernfs_drain(struct kernfs_node *kn) static void kernfs_deactivate(struct kernfs_node *kn)
{ {
struct kernfs_root *root = kernfs_root(kn); struct kernfs_root *root = kernfs_root(kn);
WARN_ON_ONCE(atomic_read(&kn->active) >= 0); BUG_ON(!(kn->flags & KERNFS_REMOVED));
atomic_add(KN_DEACTIVATED_BIAS, &kn->active);
if (kernfs_lockdep(kn)) { if (kernfs_lockdep(kn)) {
rwsem_acquire(&kn->dep_map, 0, 0, _RET_IP_); rwsem_acquire(&kn->dep_map, 0, 0, _RET_IP_);
@ -232,15 +233,13 @@ void kernfs_put(struct kernfs_node *kn)
return; return;
root = kernfs_root(kn); root = kernfs_root(kn);
repeat: repeat:
/* /* Moving/renaming is always done while holding reference.
* Moving/renaming is always done while holding reference.
* kn->parent won't change beneath us. * kn->parent won't change beneath us.
*/ */
parent = kn->parent; parent = kn->parent;
WARN_ONCE(atomic_read(&kn->active) != KN_DEACTIVATED_BIAS, WARN(!(kn->flags & KERNFS_REMOVED), "kernfs: free using entry: %s/%s\n",
"kernfs_put: %s/%s: released with incorrect active_ref %d\n", parent ? parent->name : "", kn->name);
parent ? parent->name : "", kn->name, atomic_read(&kn->active));
if (kernfs_type(kn) == KERNFS_LINK) if (kernfs_type(kn) == KERNFS_LINK)
kernfs_put(kn->symlink.target_kn); kernfs_put(kn->symlink.target_kn);
@ -282,8 +281,8 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
kn = dentry->d_fsdata; kn = dentry->d_fsdata;
mutex_lock(&kernfs_mutex); mutex_lock(&kernfs_mutex);
/* Force fresh lookup if removed */ /* The kernfs node has been deleted */
if (kn->parent && RB_EMPTY_NODE(&kn->rb)) if (kn->flags & KERNFS_REMOVED)
goto out_bad; goto out_bad;
/* The kernfs node has been moved? */ /* The kernfs node has been moved? */
@ -351,12 +350,11 @@ struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
kn->ino = ret; kn->ino = ret;
atomic_set(&kn->count, 1); atomic_set(&kn->count, 1);
atomic_set(&kn->active, KN_DEACTIVATED_BIAS); atomic_set(&kn->active, 0);
RB_CLEAR_NODE(&kn->rb);
kn->name = name; kn->name = name;
kn->mode = mode; kn->mode = mode;
kn->flags = flags; kn->flags = flags | KERNFS_REMOVED;
return kn; return kn;
@ -415,8 +413,6 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
struct kernfs_iattrs *ps_iattr; struct kernfs_iattrs *ps_iattr;
int ret; int ret;
WARN_ON_ONCE(atomic_read(&parent->active) < 0);
if (has_ns != (bool)kn->ns) { if (has_ns != (bool)kn->ns) {
WARN(1, KERN_WARNING "kernfs: ns %s in '%s' for '%s'\n", WARN(1, KERN_WARNING "kernfs: ns %s in '%s' for '%s'\n",
has_ns ? "required" : "invalid", parent->name, kn->name); has_ns ? "required" : "invalid", parent->name, kn->name);
@ -426,6 +422,9 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
if (kernfs_type(parent) != KERNFS_DIR) if (kernfs_type(parent) != KERNFS_DIR)
return -EINVAL; return -EINVAL;
if (parent->flags & KERNFS_REMOVED)
return -ENOENT;
kn->hash = kernfs_name_hash(kn->name, kn->ns); kn->hash = kernfs_name_hash(kn->name, kn->ns);
kn->parent = parent; kn->parent = parent;
kernfs_get(parent); kernfs_get(parent);
@ -442,7 +441,8 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
} }
/* Mark the entry added into directory tree */ /* Mark the entry added into directory tree */
atomic_sub(KN_DEACTIVATED_BIAS, &kn->active); kn->flags &= ~KERNFS_REMOVED;
return 0; return 0;
} }
@ -470,7 +470,7 @@ static void kernfs_remove_one(struct kernfs_addrm_cxt *acxt,
* Removal can be called multiple times on the same node. Only the * Removal can be called multiple times on the same node. Only the
* first invocation is effective and puts the base ref. * first invocation is effective and puts the base ref.
*/ */
if (atomic_read(&kn->active) < 0) if (kn->flags & KERNFS_REMOVED)
return; return;
if (kn->parent) { if (kn->parent) {
@ -484,7 +484,7 @@ static void kernfs_remove_one(struct kernfs_addrm_cxt *acxt,
} }
} }
atomic_add(KN_DEACTIVATED_BIAS, &kn->active); kn->flags |= KERNFS_REMOVED;
kn->u.removed_list = acxt->removed; kn->u.removed_list = acxt->removed;
acxt->removed = kn; acxt->removed = kn;
} }
@ -512,7 +512,7 @@ void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt)
acxt->removed = kn->u.removed_list; acxt->removed = kn->u.removed_list;
kernfs_drain(kn); kernfs_deactivate(kn);
kernfs_unmap_bin_file(kn); kernfs_unmap_bin_file(kn);
kernfs_put(kn); kernfs_put(kn);
} }
@ -610,7 +610,7 @@ struct kernfs_root *kernfs_create_root(struct kernfs_dir_ops *kdops, void *priv)
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
} }
atomic_sub(KN_DEACTIVATED_BIAS, &kn->active); kn->flags &= ~KERNFS_REMOVED;
kn->priv = priv; kn->priv = priv;
kn->dir.root = root; kn->dir.root = root;
@ -662,13 +662,9 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
kn->priv = priv; kn->priv = priv;
/* link in */ /* link in */
rc = -ENOENT; kernfs_addrm_start(&acxt);
if (kernfs_get_active(parent)) { rc = kernfs_add_one(&acxt, kn, parent);
kernfs_addrm_start(&acxt); kernfs_addrm_finish(&acxt);
rc = kernfs_add_one(&acxt, kn, parent);
kernfs_addrm_finish(&acxt);
kernfs_put_active(parent);
}
if (!rc) if (!rc)
return kn; return kn;
@ -903,29 +899,27 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent,
{ {
int error; int error;
error = -ENOENT;
if (!kernfs_get_active(new_parent))
goto out;
if (!kernfs_get_active(kn))
goto out_put_new_parent;
mutex_lock(&kernfs_mutex); mutex_lock(&kernfs_mutex);
error = -ENOENT;
if ((kn->flags | new_parent->flags) & KERNFS_REMOVED)
goto out;
error = 0; error = 0;
if ((kn->parent == new_parent) && (kn->ns == new_ns) && if ((kn->parent == new_parent) && (kn->ns == new_ns) &&
(strcmp(kn->name, new_name) == 0)) (strcmp(kn->name, new_name) == 0))
goto out_unlock; /* nothing to rename */ goto out; /* nothing to rename */
error = -EEXIST; error = -EEXIST;
if (kernfs_find_ns(new_parent, new_name, new_ns)) if (kernfs_find_ns(new_parent, new_name, new_ns))
goto out_unlock; goto out;
/* rename kernfs_node */ /* rename kernfs_node */
if (strcmp(kn->name, new_name) != 0) { if (strcmp(kn->name, new_name) != 0) {
error = -ENOMEM; error = -ENOMEM;
new_name = kstrdup(new_name, GFP_KERNEL); new_name = kstrdup(new_name, GFP_KERNEL);
if (!new_name) if (!new_name)
goto out_unlock; goto out;
if (kn->flags & KERNFS_STATIC_NAME) if (kn->flags & KERNFS_STATIC_NAME)
kn->flags &= ~KERNFS_STATIC_NAME; kn->flags &= ~KERNFS_STATIC_NAME;
@ -947,12 +941,8 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent,
kernfs_link_sibling(kn); kernfs_link_sibling(kn);
error = 0; error = 0;
out_unlock: out:
mutex_unlock(&kernfs_mutex); mutex_unlock(&kernfs_mutex);
kernfs_put_active(kn);
out_put_new_parent:
kernfs_put_active(new_parent);
out:
return error; return error;
} }
@ -972,7 +962,8 @@ static struct kernfs_node *kernfs_dir_pos(const void *ns,
struct kernfs_node *parent, loff_t hash, struct kernfs_node *pos) struct kernfs_node *parent, loff_t hash, struct kernfs_node *pos)
{ {
if (pos) { if (pos) {
int valid = pos->parent == parent && hash == pos->hash; int valid = !(pos->flags & KERNFS_REMOVED) &&
pos->parent == parent && hash == pos->hash;
kernfs_put(pos); kernfs_put(pos);
if (!valid) if (!valid)
pos = NULL; pos = NULL;

View File

@ -856,13 +856,9 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
if (ops->mmap) if (ops->mmap)
kn->flags |= KERNFS_HAS_MMAP; kn->flags |= KERNFS_HAS_MMAP;
rc = -ENOENT; kernfs_addrm_start(&acxt);
if (kernfs_get_active(parent)) { rc = kernfs_add_one(&acxt, kn, parent);
kernfs_addrm_start(&acxt); kernfs_addrm_finish(&acxt);
rc = kernfs_add_one(&acxt, kn, parent);
kernfs_addrm_finish(&acxt);
kernfs_put_active(parent);
}
if (rc) { if (rc) {
kernfs_put(kn); kernfs_put(kn);

View File

@ -26,8 +26,7 @@ struct kernfs_iattrs {
struct simple_xattrs xattrs; struct simple_xattrs xattrs;
}; };
/* +1 to avoid triggering overflow warning when negating it */ #define KN_DEACTIVATED_BIAS INT_MIN
#define KN_DEACTIVATED_BIAS (INT_MIN + 1)
/* KERNFS_TYPE_MASK and types are defined in include/linux/kernfs.h */ /* KERNFS_TYPE_MASK and types are defined in include/linux/kernfs.h */

View File

@ -40,13 +40,9 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
kn->symlink.target_kn = target; kn->symlink.target_kn = target;
kernfs_get(target); /* ref owned by symlink */ kernfs_get(target); /* ref owned by symlink */
error = -ENOENT; kernfs_addrm_start(&acxt);
if (kernfs_get_active(parent)) { error = kernfs_add_one(&acxt, kn, parent);
kernfs_addrm_start(&acxt); kernfs_addrm_finish(&acxt);
error = kernfs_add_one(&acxt, kn, parent);
kernfs_addrm_finish(&acxt);
kernfs_put_active(parent);
}
if (!error) if (!error)
return kn; return kn;

View File

@ -37,6 +37,7 @@ enum kernfs_node_type {
#define KERNFS_FLAG_MASK ~KERNFS_TYPE_MASK #define KERNFS_FLAG_MASK ~KERNFS_TYPE_MASK
enum kernfs_node_flag { enum kernfs_node_flag {
KERNFS_REMOVED = 0x0010,
KERNFS_NS = 0x0020, KERNFS_NS = 0x0020,
KERNFS_HAS_SEQ_SHOW = 0x0040, KERNFS_HAS_SEQ_SHOW = 0x0040,
KERNFS_HAS_MMAP = 0x0080, KERNFS_HAS_MMAP = 0x0080,