summaryrefslogtreecommitdiff
path: root/fs/kernfs
diff options
context:
space:
mode:
authorGreg Kroah-Hartman <gregkh@linuxfoundation.org>2014-01-14 02:30:47 +0400
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2014-01-14 02:30:47 +0400
commit4f4b1b6471cf219d136776f9ff9631a07c4e92b5 (patch)
treee70651b3459b90062cf59797a57e5d90e88e4166 /fs/kernfs
parent55f6e30d0a6a8975cc0831e8a4a3715b815b6a2f (diff)
downloadlinux-4f4b1b6471cf219d136776f9ff9631a07c4e92b5.tar.xz
Revert "kernfs: restructure removal path to fix possible premature return"
This reverts commit 45a140e587f3d32d8d424ed940dffb61e1739047. 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>
Diffstat (limited to 'fs/kernfs')
-rw-r--r--fs/kernfs/dir.c139
1 files changed, 53 insertions, 86 deletions
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index e565ec096ae9..7f8afc1d08f1 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -181,38 +181,14 @@ void kernfs_put_active(struct kernfs_node *kn)
* kernfs_drain - drain kernfs_node
* @kn: kernfs_node to drain
*
- * Drain existing usages of @kn. Mutiple removers may invoke this function
- * concurrently on @kn and all will return after draining is complete.
- * Returns %true if drain is performed and kernfs_mutex was temporarily
- * released. %false if @kn was already drained and no operation was
- * necessary.
- *
- * The caller is responsible for ensuring @kn stays pinned while this
- * function is in progress even if it gets removed by someone else.
+ * Drain existing usages.
*/
-static bool kernfs_drain(struct kernfs_node *kn)
- __releases(&kernfs_mutex) __acquires(&kernfs_mutex)
+static void kernfs_drain(struct kernfs_node *kn)
{
struct kernfs_root *root = kernfs_root(kn);
- lockdep_assert_held(&kernfs_mutex);
WARN_ON_ONCE(atomic_read(&kn->active) >= 0);
- /*
- * We want to go through the active ref lockdep annotation at least
- * once for all node removals, but the lockdep annotation can't be
- * nested inside kernfs_mutex and deactivation can't make forward
- * progress if we keep dropping the mutex. Use JUST_ACTIVATED to
- * force the slow path once for each deactivation if lockdep is
- * enabled.
- */
- if ((!kernfs_lockdep(kn) || !(kn->flags & KERNFS_JUST_DEACTIVATED)) &&
- atomic_read(&kn->active) == KN_DEACTIVATED_BIAS)
- return false;
-
- kn->flags &= ~KERNFS_JUST_DEACTIVATED;
- mutex_unlock(&kernfs_mutex);
-
if (kernfs_lockdep(kn)) {
rwsem_acquire(&kn->dep_map, 0, 0, _RET_IP_);
if (atomic_read(&kn->active) != KN_DEACTIVATED_BIAS)
@@ -226,9 +202,6 @@ static bool kernfs_drain(struct kernfs_node *kn)
lock_acquired(&kn->dep_map, _RET_IP_);
rwsem_release(&kn->dep_map, 1, _RET_IP_);
}
-
- mutex_lock(&kernfs_mutex);
- return true;
}
/**
@@ -474,6 +447,49 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
}
/**
+ * kernfs_remove_one - remove kernfs_node from parent
+ * @acxt: addrm context to use
+ * @kn: kernfs_node to be removed
+ *
+ * Mark @kn removed and drop nlink of parent inode if @kn is a
+ * directory. @kn is unlinked from the children list.
+ *
+ * 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().
+ */
+static void kernfs_remove_one(struct kernfs_addrm_cxt *acxt,
+ struct kernfs_node *kn)
+{
+ struct kernfs_iattrs *ps_iattr;
+
+ /*
+ * Removal can be called multiple times on the same node. Only the
+ * first invocation is effective and puts the base ref.
+ */
+ if (atomic_read(&kn->active) < 0)
+ return;
+
+ if (kn->parent) {
+ kernfs_unlink_sibling(kn);
+
+ /* Update timestamps on the parent */
+ ps_iattr = kn->parent->iattr;
+ if (ps_iattr) {
+ ps_iattr->ia_iattr.ia_ctime = CURRENT_TIME;
+ ps_iattr->ia_iattr.ia_mtime = CURRENT_TIME;
+ }
+ }
+
+ atomic_add(KN_DEACTIVATED_BIAS, &kn->active);
+ kn->u.removed_list = acxt->removed;
+ acxt->removed = kn;
+}
+
+/**
* kernfs_addrm_finish - finish up kernfs_node add/remove
* @acxt: addrm context to finish up
*
@@ -496,6 +512,7 @@ void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt)
acxt->removed = kn->u.removed_list;
+ kernfs_drain(kn);
kernfs_unmap_bin_file(kn);
kernfs_put(kn);
}
@@ -805,73 +822,23 @@ static struct kernfs_node *kernfs_next_descendant_post(struct kernfs_node *pos,
return pos->parent;
}
-static void __kernfs_deactivate(struct kernfs_node *kn)
-{
- struct kernfs_node *pos;
-
- lockdep_assert_held(&kernfs_mutex);
-
- /* prevent any new usage under @kn by deactivating all nodes */
- pos = NULL;
- while ((pos = kernfs_next_descendant_post(pos, kn))) {
- if (atomic_read(&pos->active) >= 0) {
- atomic_add(KN_DEACTIVATED_BIAS, &pos->active);
- pos->flags |= KERNFS_JUST_DEACTIVATED;
- }
- }
-
- /*
- * Drain the subtree. If kernfs_drain() blocked to drain, which is
- * indicated by %true return, it temporarily released kernfs_mutex
- * and the rbtree might have been modified inbetween breaking our
- * future walk. Restart the walk after each %true return.
- */
- pos = NULL;
- while ((pos = kernfs_next_descendant_post(pos, kn))) {
- bool drained;
-
- kernfs_get(pos);
- drained = kernfs_drain(pos);
- kernfs_put(pos);
- if (drained)
- pos = NULL;
- }
-}
-
static void __kernfs_remove(struct kernfs_addrm_cxt *acxt,
struct kernfs_node *kn)
{
- struct kernfs_node *pos;
-
- lockdep_assert_held(&kernfs_mutex);
+ struct kernfs_node *pos, *next;
if (!kn)
return;
pr_debug("kernfs %s: removing\n", kn->name);
- __kernfs_deactivate(kn);
-
- /* unlink the subtree node-by-node */
+ next = NULL;
do {
- struct kernfs_iattrs *ps_iattr;
-
- pos = kernfs_leftmost_descendant(kn);
-
- if (pos->parent) {
- kernfs_unlink_sibling(pos);
-
- /* update timestamps on the parent */
- ps_iattr = pos->parent->iattr;
- if (ps_iattr) {
- ps_iattr->ia_iattr.ia_ctime = CURRENT_TIME;
- ps_iattr->ia_iattr.ia_mtime = CURRENT_TIME;
- }
- }
-
- pos->u.removed_list = acxt->removed;
- acxt->removed = pos;
- } while (pos != kn);
+ pos = next;
+ next = kernfs_next_descendant_post(pos, kn);
+ if (pos)
+ kernfs_remove_one(acxt, pos);
+ } while (next);
}
/**