Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f601f9a2 authored by Tejun Heo's avatar Tejun Heo Committed by Greg Kroah-Hartman
Browse files

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: default avatarTejun Heo <tj@kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 45a140e5

fs/kernfs/dir.c

+33 −9
Original line number Original line Diff line number Diff line
@@ -121,13 +121,17 @@ static int kernfs_link_sibling(struct kernfs_node *kn) 
 *	Locking:

 *	Locking:

 *	mutex_lock(kernfs_mutex)

 *	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)

	if (kernfs_type(kn) == KERNFS_DIR)

		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);

	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;

		acxt->removed = kn->u.removed_list;





		kernfs_unmap_bin_file(kn);

		kernfs_put(kn);

		kernfs_put(kn);

	}

	}

}

}
@@ -854,23 +857,44 @@ static void __kernfs_remove(struct kernfs_addrm_cxt *acxt, 




	/* unlink the subtree node-by-node */

	/* unlink the subtree node-by-node */

	do {

	do {

		struct kernfs_iattrs *ps_iattr;



		pos = kernfs_leftmost_descendant(kn);

		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 */

			/* update timestamps on the parent */

			ps_iattr = pos->parent->iattr;

			if (ps_iattr) {

			if (ps_iattr) {

				ps_iattr->ia_iattr.ia_ctime = CURRENT_TIME;

				ps_iattr->ia_iattr.ia_ctime = CURRENT_TIME;

				ps_iattr->ia_iattr.ia_mtime = CURRENT_TIME;

				ps_iattr->ia_iattr.ia_mtime = CURRENT_TIME;

			}

			}

		}





			pos->u.removed_list = acxt->removed;

			pos->u.removed_list = acxt->removed;

			acxt->removed = pos;

			acxt->removed = pos;

		}



		kernfs_put(pos);

	} while (pos != kn);

	} while (pos != kn);

}

}

fs/kernfs/file.c

+1 −4
Original line number Original line Diff line number Diff line
@@ -700,14 +700,11 @@ static int kernfs_fop_release(struct inode *inode, struct file *filp) 
	return 0;

	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_node *on;

	struct kernfs_open_file *of;

	struct kernfs_open_file *of;





	if (!(kn->flags & KERNFS_HAS_MMAP))

		return;



	spin_lock_irq(&kernfs_open_node_lock);

	spin_lock_irq(&kernfs_open_node_lock);

	on = kn->attr.open;

	on = kn->attr.open;

	if (on)

	if (on)

fs/kernfs/kernfs-internal.h

+1 −1
Original line number Original line Diff line number Diff line
@@ -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;

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

 * symlink.c