Revert "kernfs: remove kernfs_addrm_cxt"

This reverts commit 99177a3411.

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:20:56 -08:00
parent f4b3e631b3
commit 7653fe9d6c
5 changed files with 121 additions and 29 deletions

View File

@ -398,8 +398,29 @@ struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
return NULL;
}
/**
* kernfs_addrm_start - prepare for kernfs_node add/remove
* @acxt: pointer to kernfs_addrm_cxt to be used
*
* This function is called when the caller is about to add or remove
* kernfs_node. This function acquires kernfs_mutex. @acxt is used
* to keep and pass context to other addrm functions.
*
* LOCKING:
* Kernel thread context (may sleep). kernfs_mutex is locked on
* return.
*/
void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt)
__acquires(kernfs_mutex)
{
memset(acxt, 0, sizeof(*acxt));
mutex_lock(&kernfs_mutex);
}
/**
* kernfs_add_one - add kernfs_node to parent without warning
* @acxt: addrm context to use
* @kn: kernfs_node to be added
* @parent: the parent kernfs_node to add @kn to
*
@ -407,29 +428,34 @@ struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
* parent inode if @kn is a directory and link into the children list
* of the parent.
*
* This function should be called between calls to
* kernfs_addrm_start() and kernfs_addrm_finish() and should be passed
* the same @acxt as passed to kernfs_addrm_start().
*
* LOCKING:
* Determined by kernfs_addrm_start().
*
* RETURNS:
* 0 on success, -EEXIST if entry with the given name already
* exists.
*/
int kernfs_add_one(struct kernfs_node *kn, struct kernfs_node *parent)
int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
struct kernfs_node *parent)
{
bool has_ns = kernfs_ns_enabled(parent);
struct kernfs_iattrs *ps_iattr;
bool has_ns;
int ret;
if (!kernfs_get_active(parent))
return -ENOENT;
WARN_ON_ONCE(atomic_read(&parent->active) < 0);
mutex_lock(&kernfs_mutex);
ret = -EINVAL;
has_ns = kernfs_ns_enabled(parent);
if (WARN(has_ns != (bool)kn->ns, KERN_WARNING "kernfs: ns %s in '%s' for '%s'\n",
has_ns ? "required" : "invalid", parent->name, kn->name))
goto out_unlock;
if (has_ns != (bool)kn->ns) {
WARN(1, KERN_WARNING "kernfs: ns %s in '%s' for '%s'\n",
has_ns ? "required" : "invalid", parent->name, kn->name);
return -EINVAL;
}
if (kernfs_type(parent) != KERNFS_DIR)
goto out_unlock;
return -EINVAL;
kn->hash = kernfs_name_hash(kn->name, kn->ns);
kn->parent = parent;
@ -437,7 +463,7 @@ int kernfs_add_one(struct kernfs_node *kn, struct kernfs_node *parent)
ret = kernfs_link_sibling(kn);
if (ret)
goto out_unlock;
return ret;
/* Update timestamps on the parent */
ps_iattr = parent->iattr;
@ -448,11 +474,34 @@ int kernfs_add_one(struct kernfs_node *kn, struct kernfs_node *parent)
/* Mark the entry added into directory tree */
atomic_sub(KN_DEACTIVATED_BIAS, &kn->active);
ret = 0;
out_unlock:
return 0;
}
/**
* kernfs_addrm_finish - finish up kernfs_node add/remove
* @acxt: addrm context to finish up
*
* Finish up kernfs_node add/remove. Resources acquired by
* kernfs_addrm_start() are released and removed kernfs_nodes are
* cleaned up.
*
* LOCKING:
* kernfs_mutex is released.
*/
void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt)
__releases(kernfs_mutex)
{
/* release resources acquired by kernfs_addrm_start() */
mutex_unlock(&kernfs_mutex);
kernfs_put_active(parent);
return ret;
/* kill removed kernfs_nodes */
while (acxt->removed) {
struct kernfs_node *kn = acxt->removed;
acxt->removed = kn->u.removed_list;
kernfs_put(kn);
}
}
/**
@ -584,6 +633,7 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
const char *name, umode_t mode,
void *priv, const void *ns)
{
struct kernfs_addrm_cxt acxt;
struct kernfs_node *kn;
int rc;
@ -598,7 +648,14 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
kn->priv = priv;
/* link in */
rc = kernfs_add_one(kn, parent);
rc = -ENOENT;
if (kernfs_get_active(parent)) {
kernfs_addrm_start(&acxt);
rc = kernfs_add_one(&acxt, kn, parent);
kernfs_addrm_finish(&acxt);
kernfs_put_active(parent);
}
if (!rc)
return kn;
@ -784,7 +841,8 @@ static void __kernfs_deactivate(struct kernfs_node *kn)
}
}
static void __kernfs_remove(struct kernfs_node *kn)
static void __kernfs_remove(struct kernfs_addrm_cxt *acxt,
struct kernfs_node *kn)
{
struct kernfs_node *pos;
@ -832,7 +890,8 @@ static void __kernfs_remove(struct kernfs_node *kn)
ps_iattr->ia_iattr.ia_mtime = CURRENT_TIME;
}
kernfs_put(pos);
pos->u.removed_list = acxt->removed;
acxt->removed = pos;
}
kernfs_put(pos);
@ -847,9 +906,11 @@ static void __kernfs_remove(struct kernfs_node *kn)
*/
void kernfs_remove(struct kernfs_node *kn)
{
mutex_lock(&kernfs_mutex);
__kernfs_remove(kn);
mutex_unlock(&kernfs_mutex);
struct kernfs_addrm_cxt acxt;
kernfs_addrm_start(&acxt);
__kernfs_remove(&acxt, kn);
kernfs_addrm_finish(&acxt);
}
/**
@ -864,6 +925,7 @@ void kernfs_remove(struct kernfs_node *kn)
int kernfs_remove_by_name_ns(struct kernfs_node *parent, const char *name,
const void *ns)
{
struct kernfs_addrm_cxt acxt;
struct kernfs_node *kn;
if (!parent) {
@ -872,13 +934,13 @@ int kernfs_remove_by_name_ns(struct kernfs_node *parent, const char *name,
return -ENOENT;
}
mutex_lock(&kernfs_mutex);
kernfs_addrm_start(&acxt);
kn = kernfs_find_ns(parent, name, ns);
if (kn)
__kernfs_remove(kn);
__kernfs_remove(&acxt, kn);
mutex_unlock(&kernfs_mutex);
kernfs_addrm_finish(&acxt);
if (kn)
return 0;

View File

@ -817,6 +817,7 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
bool name_is_static,
struct lock_class_key *key)
{
struct kernfs_addrm_cxt acxt;
struct kernfs_node *kn;
unsigned flags;
int rc;
@ -852,7 +853,14 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
if (ops->mmap)
kn->flags |= KERNFS_HAS_MMAP;
rc = kernfs_add_one(kn, parent);
rc = -ENOENT;
if (kernfs_get_active(parent)) {
kernfs_addrm_start(&acxt);
rc = kernfs_add_one(&acxt, kn, parent);
kernfs_addrm_finish(&acxt);
kernfs_put_active(parent);
}
if (rc) {
kernfs_put(kn);
return ERR_PTR(rc);

View File

@ -45,6 +45,13 @@ static inline struct kernfs_root *kernfs_root(struct kernfs_node *kn)
return kn->dir.root;
}
/*
* Context structure to be used while adding/removing nodes.
*/
struct kernfs_addrm_cxt {
struct kernfs_node *removed;
};
/*
* mount.c
*/
@ -94,7 +101,10 @@ extern const struct inode_operations kernfs_dir_iops;
struct kernfs_node *kernfs_get_active(struct kernfs_node *kn);
void kernfs_put_active(struct kernfs_node *kn);
int kernfs_add_one(struct kernfs_node *kn, struct kernfs_node *parent);
void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt);
int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
struct kernfs_node *parent);
void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt);
struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
umode_t mode, unsigned flags);

View File

@ -27,6 +27,7 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
struct kernfs_node *target)
{
struct kernfs_node *kn;
struct kernfs_addrm_cxt acxt;
int error;
kn = kernfs_new_node(kernfs_root(parent), name, S_IFLNK|S_IRWXUGO,
@ -39,7 +40,14 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
kn->symlink.target_kn = target;
kernfs_get(target); /* ref owned by symlink */
error = kernfs_add_one(kn, parent);
error = -ENOENT;
if (kernfs_get_active(parent)) {
kernfs_addrm_start(&acxt);
error = kernfs_add_one(&acxt, kn, parent);
kernfs_addrm_finish(&acxt);
kernfs_put_active(parent);
}
if (!error)
return kn;

View File

@ -89,6 +89,10 @@ struct kernfs_node {
struct rb_node rb;
union {
struct kernfs_node *removed_list;
} u;
const void *ns; /* namespace tag */
unsigned int hash; /* ns + name hash */
union {