summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTejun Heo <tj@kernel.org>2014-01-10 08:57:23 -0500
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2014-01-10 13:48:08 -0800
commitf601f9a2bf7dc1f7ee18feece4c4e2fc6845d6c4 (patch)
treef8213b3d7542c85c42cb1a20668098609d65762c
parent45a140e587f3d32d8d424ed940dffb61e1739047 (diff)
downloadlinux-f601f9a2bf7dc1f7ee18feece4c4e2fc6845d6c4.tar.bz2
kernfs: invoke kernfs_unmap_bin_file() directly from __kernfs_remove()
kernfs_unmap_bin_file() is supposed to unmap all memory mappings of the target file before kernfs_remove() finishes; however, it currently is being called from kernfs_addrm_finish() and has the same race problem as the original implementation of deactivation when there are multiple removers - only the remover which snatches the node to its addrm_cxt->removed list is guaranteed to wait for its completion before returning. It can be fixed by moving kernfs_unmap_bin_file() invocation from kernfs_addrm_finish() to __kernfs_remove(). The function may be called multiple times but that shouldn't do any harm. We end up dropping kernfs_mutex in the removal loop and the node may be removed inbetween by someone else. kernfs_unlink_sibling() is updated to test whether the node has already been removed and return accordingly. __kernfs_remove() in turn performs post-unlinking cleanup only if it actually unlinked the node. KERNFS_HAS_MMAP test is moved out of the unmap function into __kernfs_remove() so that we don't unlock kernfs_mutex unnecessarily. While at it, drop the now meaningless "bin" qualifier from the function name. v2: Rewritten to fit the v2 restructuring of removal path. HAS_MMAP test relocated. Signed-off-by: Tejun Heo <tj@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--fs/kernfs/dir.c42
-rw-r--r--fs/kernfs/file.c5
-rw-r--r--fs/kernfs/kernfs-internal.h2
3 files changed, 35 insertions, 14 deletions
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index e565ec096ae9..f878e4f2efe7 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -121,13 +121,17 @@ static int kernfs_link_sibling(struct kernfs_node *kn)
* Locking:
* mutex_lock(kernfs_mutex)
*/
-static void kernfs_unlink_sibling(struct kernfs_node *kn)
+static bool kernfs_unlink_sibling(struct kernfs_node *kn)
{
+ if (RB_EMPTY_NODE(&kn->rb))
+ return false;
+
if (kernfs_type(kn) == KERNFS_DIR)
kn->parent->dir.subdirs--;
rb_erase(&kn->rb, &kn->parent->dir.children);
RB_CLEAR_NODE(&kn->rb);
+ return true;
}
/**
@@ -496,7 +500,6 @@ void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt)
acxt->removed = kn->u.removed_list;
- kernfs_unmap_bin_file(kn);
kernfs_put(kn);
}
}
@@ -854,23 +857,44 @@ static void __kernfs_remove(struct kernfs_addrm_cxt *acxt,
/* unlink the subtree node-by-node */
do {
- struct kernfs_iattrs *ps_iattr;
-
pos = kernfs_leftmost_descendant(kn);
- if (pos->parent) {
- kernfs_unlink_sibling(pos);
+ /*
+ * We're gonna release kernfs_mutex to unmap bin files,
+ * Make sure @pos doesn't go away inbetween.
+ */
+ kernfs_get(pos);
+
+ /*
+ * This must be come before unlinking; otherwise, when
+ * there are multiple removers, some may finish before
+ * unmapping is complete.
+ */
+ if (pos->flags & KERNFS_HAS_MMAP) {
+ mutex_unlock(&kernfs_mutex);
+ kernfs_unmap_file(pos);
+ mutex_lock(&kernfs_mutex);
+ }
+
+ /*
+ * kernfs_unlink_sibling() succeeds once per node. Use it
+ * to decide who's responsible for cleanups.
+ */
+ if (!pos->parent || kernfs_unlink_sibling(pos)) {
+ struct kernfs_iattrs *ps_iattr =
+ pos->parent ? pos->parent->iattr : NULL;
/* 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;
}
- pos->u.removed_list = acxt->removed;
- acxt->removed = pos;
+ kernfs_put(pos);
} while (pos != kn);
}
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 231a171f48b6..404ffd2f27bc 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -700,14 +700,11 @@ static int kernfs_fop_release(struct inode *inode, struct file *filp)
return 0;
}
-void kernfs_unmap_bin_file(struct kernfs_node *kn)
+void kernfs_unmap_file(struct kernfs_node *kn)
{
struct kernfs_open_node *on;
struct kernfs_open_file *of;
- if (!(kn->flags & KERNFS_HAS_MMAP))
- return;
-
spin_lock_irq(&kernfs_open_node_lock);
on = kn->attr.open;
if (on)
diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
index 57a93f4d645c..e9ec38c86074 100644
--- a/fs/kernfs/kernfs-internal.h
+++ b/fs/kernfs/kernfs-internal.h
@@ -113,7 +113,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
*/
extern const struct file_operations kernfs_file_fops;
-void kernfs_unmap_bin_file(struct kernfs_node *kn);
+void kernfs_unmap_file(struct kernfs_node *kn);
/*
* symlink.c