Skip to content

Commit

Permalink
kernfs: switch kernfs to use an rwsem
Browse files Browse the repository at this point in the history 
The kernfs global lock restricts the ability to perform kernfs node
lookup operations in parallel during path walks.

Change the kernfs mutex to an rwsem so that, when opportunity arises,
node searches can be done in parallel with path walk lookups.

Reviewed-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Ian Kent <raven@themaw.net>
Link: https://lore.kernel.org/r/162642770946.63632.2218304587223241374.stgit@web.messagingengine.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Ian Kent authored and Greg Kroah-Hartman committed Jul 27, 2021
1 parent c7e7c04 commit 7ba0273
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 71 deletions.
100 changes: 50 additions & 50 deletions fs/kernfs/dir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

#include "kernfs-internal.h"

DEFINE_MUTEX(kernfs_mutex);
DECLARE_RWSEM(kernfs_rwsem);
static DEFINE_SPINLOCK(kernfs_rename_lock); /* kn->parent and ->name */
static char kernfs_pr_cont_buf[PATH_MAX]; /* protected by rename_lock */
static DEFINE_SPINLOCK(kernfs_idr_lock); /* root->ino_idr */
Expand All @@ -26,7 +26,7 @@ static DEFINE_SPINLOCK(kernfs_idr_lock); /* root->ino_idr */

static bool kernfs_active(struct kernfs_node *kn)
{
lockdep_assert_held(&kernfs_mutex);
lockdep_assert_held(&kernfs_rwsem);
return atomic_read(&kn->active) >= 0;
}

Expand Down Expand Up @@ -340,7 +340,7 @@ static int kernfs_sd_compare(const struct kernfs_node *left,
* @kn->parent->dir.children.
*
* Locking:
* mutex_lock(kernfs_mutex)
* kernfs_rwsem held exclusive
*
* RETURNS:
* 0 on susccess -EEXIST on failure.
Expand Down Expand Up @@ -386,7 +386,7 @@ static int kernfs_link_sibling(struct kernfs_node *kn)
* removed, %false if @kn wasn't on the rbtree.
*
* Locking:
* mutex_lock(kernfs_mutex)
* kernfs_rwsem held exclusive
*/
static bool kernfs_unlink_sibling(struct kernfs_node *kn)
{
Expand Down Expand Up @@ -457,14 +457,14 @@ void kernfs_put_active(struct kernfs_node *kn)
* return after draining is complete.
*/
static void kernfs_drain(struct kernfs_node *kn)
__releases(&kernfs_mutex) __acquires(&kernfs_mutex)
__releases(&kernfs_rwsem) __acquires(&kernfs_rwsem)
{
struct kernfs_root *root = kernfs_root(kn);

lockdep_assert_held(&kernfs_mutex);
lockdep_assert_held_write(&kernfs_rwsem);
WARN_ON_ONCE(kernfs_active(kn));

mutex_unlock(&kernfs_mutex);
up_write(&kernfs_rwsem);

if (kernfs_lockdep(kn)) {
rwsem_acquire(&kn->dep_map, 0, 0, _RET_IP_);
Expand All @@ -483,7 +483,7 @@ static void kernfs_drain(struct kernfs_node *kn)

kernfs_drain_open_files(kn);

mutex_lock(&kernfs_mutex);
down_write(&kernfs_rwsem);
}

/**
Expand Down Expand Up @@ -722,7 +722,7 @@ int kernfs_add_one(struct kernfs_node *kn)
bool has_ns;
int ret;

mutex_lock(&kernfs_mutex);
down_write(&kernfs_rwsem);

ret = -EINVAL;
has_ns = kernfs_ns_enabled(parent);
Expand Down Expand Up @@ -753,7 +753,7 @@ int kernfs_add_one(struct kernfs_node *kn)
ps_iattr->ia_mtime = ps_iattr->ia_ctime;
}

mutex_unlock(&kernfs_mutex);
up_write(&kernfs_rwsem);

/*
* Activate the new node unless CREATE_DEACTIVATED is requested.
Expand All @@ -767,7 +767,7 @@ int kernfs_add_one(struct kernfs_node *kn)
return 0;

out_unlock:
mutex_unlock(&kernfs_mutex);
up_write(&kernfs_rwsem);
return ret;
}

Expand All @@ -788,7 +788,7 @@ static struct kernfs_node *kernfs_find_ns(struct kernfs_node *parent,
bool has_ns = kernfs_ns_enabled(parent);
unsigned int hash;

lockdep_assert_held(&kernfs_mutex);
lockdep_assert_held(&kernfs_rwsem);

if (has_ns != (bool)ns) {
WARN(1, KERN_WARNING "kernfs: ns %s in '%s' for '%s'\n",
Expand Down Expand Up @@ -820,7 +820,7 @@ static struct kernfs_node *kernfs_walk_ns(struct kernfs_node *parent,
size_t len;
char *p, *name;

lockdep_assert_held(&kernfs_mutex);
lockdep_assert_held_read(&kernfs_rwsem);

/* grab kernfs_rename_lock to piggy back on kernfs_pr_cont_buf */
spin_lock_irq(&kernfs_rename_lock);
Expand Down Expand Up @@ -860,10 +860,10 @@ struct kernfs_node *kernfs_find_and_get_ns(struct kernfs_node *parent,
{
struct kernfs_node *kn;

mutex_lock(&kernfs_mutex);
down_read(&kernfs_rwsem);
kn = kernfs_find_ns(parent, name, ns);
kernfs_get(kn);
mutex_unlock(&kernfs_mutex);
up_read(&kernfs_rwsem);

return kn;
}
Expand All @@ -884,10 +884,10 @@ struct kernfs_node *kernfs_walk_and_get_ns(struct kernfs_node *parent,
{
struct kernfs_node *kn;

mutex_lock(&kernfs_mutex);
down_read(&kernfs_rwsem);
kn = kernfs_walk_ns(parent, path, ns);
kernfs_get(kn);
mutex_unlock(&kernfs_mutex);
up_read(&kernfs_rwsem);

return kn;
}
Expand Down Expand Up @@ -1046,18 +1046,18 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
/* If the kernfs parent node has changed discard and
* proceed to ->lookup.
*/
mutex_lock(&kernfs_mutex);
down_read(&kernfs_rwsem);
spin_lock(&dentry->d_lock);
parent = kernfs_dentry_node(dentry->d_parent);
if (parent) {
if (kernfs_dir_changed(parent, dentry)) {
spin_unlock(&dentry->d_lock);
mutex_unlock(&kernfs_mutex);
up_read(&kernfs_rwsem);
return 0;
}
}
spin_unlock(&dentry->d_lock);
mutex_unlock(&kernfs_mutex);
up_read(&kernfs_rwsem);

/* The kernfs parent node hasn't changed, leave the
* dentry negative and return success.
Expand All @@ -1066,7 +1066,7 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
}

kn = kernfs_dentry_node(dentry);
mutex_lock(&kernfs_mutex);
down_read(&kernfs_rwsem);

/* The kernfs node has been deactivated */
if (!kernfs_active(kn))
Expand All @@ -1085,10 +1085,10 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
kernfs_info(dentry->d_sb)->ns != kn->ns)
goto out_bad;

mutex_unlock(&kernfs_mutex);
up_read(&kernfs_rwsem);
return 1;
out_bad:
mutex_unlock(&kernfs_mutex);
up_read(&kernfs_rwsem);
return 0;
}

Expand All @@ -1106,7 +1106,7 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir,
struct inode *inode = NULL;
const void *ns = NULL;

mutex_lock(&kernfs_mutex);
down_read(&kernfs_rwsem);
if (kernfs_ns_enabled(parent))
ns = kernfs_info(dir->i_sb)->ns;

Expand All @@ -1122,7 +1122,7 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir,
kernfs_set_rev(parent, dentry);
/* instantiate and hash (possibly negative) dentry */
ret = d_splice_alias(inode, dentry);
mutex_unlock(&kernfs_mutex);
up_read(&kernfs_rwsem);

return ret;
}
Expand Down Expand Up @@ -1244,7 +1244,7 @@ static struct kernfs_node *kernfs_next_descendant_post(struct kernfs_node *pos,
{
struct rb_node *rbn;

lockdep_assert_held(&kernfs_mutex);
lockdep_assert_held_write(&kernfs_rwsem);

/* if first iteration, visit leftmost descendant which may be root */
if (!pos)
Expand Down Expand Up @@ -1280,7 +1280,7 @@ void kernfs_activate(struct kernfs_node *kn)
{
struct kernfs_node *pos;

mutex_lock(&kernfs_mutex);
down_write(&kernfs_rwsem);

pos = NULL;
while ((pos = kernfs_next_descendant_post(pos, kn))) {
Expand All @@ -1294,14 +1294,14 @@ void kernfs_activate(struct kernfs_node *kn)
pos->flags |= KERNFS_ACTIVATED;
}

mutex_unlock(&kernfs_mutex);
up_write(&kernfs_rwsem);
}

static void __kernfs_remove(struct kernfs_node *kn)
{
struct kernfs_node *pos;

lockdep_assert_held(&kernfs_mutex);
lockdep_assert_held_write(&kernfs_rwsem);

/*
* Short-circuit if non-root @kn has already finished removal.
Expand All @@ -1324,7 +1324,7 @@ static void __kernfs_remove(struct kernfs_node *kn)
pos = kernfs_leftmost_descendant(kn);

/*
* kernfs_drain() drops kernfs_mutex temporarily and @pos's
* kernfs_drain() drops kernfs_rwsem temporarily and @pos's
* base ref could have been put by someone else by the time
* the function returns. Make sure it doesn't go away
* underneath us.
Expand Down Expand Up @@ -1371,9 +1371,9 @@ static void __kernfs_remove(struct kernfs_node *kn)
*/
void kernfs_remove(struct kernfs_node *kn)
{
mutex_lock(&kernfs_mutex);
down_write(&kernfs_rwsem);
__kernfs_remove(kn);
mutex_unlock(&kernfs_mutex);
up_write(&kernfs_rwsem);
}

/**
Expand Down Expand Up @@ -1460,17 +1460,17 @@ bool kernfs_remove_self(struct kernfs_node *kn)
{
bool ret;

mutex_lock(&kernfs_mutex);
down_write(&kernfs_rwsem);
kernfs_break_active_protection(kn);

/*
* SUICIDAL is used to arbitrate among competing invocations. Only
* the first one will actually perform removal. When the removal
* is complete, SUICIDED is set and the active ref is restored
* while holding kernfs_mutex. The ones which lost arbitration
* waits for SUICDED && drained which can happen only after the
* enclosing kernfs operation which executed the winning instance
* of kernfs_remove_self() finished.
* while kernfs_rwsem for held exclusive. The ones which lost
* arbitration waits for SUICIDED && drained which can happen only
* after the enclosing kernfs operation which executed the winning
* instance of kernfs_remove_self() finished.
*/
if (!(kn->flags & KERNFS_SUICIDAL)) {
kn->flags |= KERNFS_SUICIDAL;
Expand All @@ -1488,22 +1488,22 @@ bool kernfs_remove_self(struct kernfs_node *kn)
atomic_read(&kn->active) == KN_DEACTIVATED_BIAS)
break;

mutex_unlock(&kernfs_mutex);
up_write(&kernfs_rwsem);
schedule();
mutex_lock(&kernfs_mutex);
down_write(&kernfs_rwsem);
}
finish_wait(waitq, &wait);
WARN_ON_ONCE(!RB_EMPTY_NODE(&kn->rb));
ret = false;
}

/*
* This must be done while holding kernfs_mutex; otherwise, waiting
* for SUICIDED && deactivated could finish prematurely.
* This must be done while kernfs_rwsem held exclusive; otherwise,
* waiting for SUICIDED && deactivated could finish prematurely.
*/
kernfs_unbreak_active_protection(kn);

mutex_unlock(&kernfs_mutex);
up_write(&kernfs_rwsem);
return ret;
}

Expand All @@ -1527,13 +1527,13 @@ int kernfs_remove_by_name_ns(struct kernfs_node *parent, const char *name,
return -ENOENT;
}

mutex_lock(&kernfs_mutex);
down_write(&kernfs_rwsem);

kn = kernfs_find_ns(parent, name, ns);
if (kn)
__kernfs_remove(kn);

mutex_unlock(&kernfs_mutex);
up_write(&kernfs_rwsem);

if (kn)
return 0;
Expand All @@ -1559,7 +1559,7 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent,
if (!kn->parent)
return -EINVAL;

mutex_lock(&kernfs_mutex);
down_write(&kernfs_rwsem);

error = -ENOENT;
if (!kernfs_active(kn) || !kernfs_active(new_parent) ||
Expand Down Expand Up @@ -1613,7 +1613,7 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent,

error = 0;
out:
mutex_unlock(&kernfs_mutex);
up_write(&kernfs_rwsem);
return error;
}

Expand Down Expand Up @@ -1688,7 +1688,7 @@ static int kernfs_fop_readdir(struct file *file, struct dir_context *ctx)

if (!dir_emit_dots(file, ctx))
return 0;
mutex_lock(&kernfs_mutex);
down_read(&kernfs_rwsem);

if (kernfs_ns_enabled(parent))
ns = kernfs_info(dentry->d_sb)->ns;
Expand All @@ -1705,12 +1705,12 @@ static int kernfs_fop_readdir(struct file *file, struct dir_context *ctx)
file->private_data = pos;
kernfs_get(pos);

mutex_unlock(&kernfs_mutex);
up_read(&kernfs_rwsem);
if (!dir_emit(ctx, name, len, ino, type))
return 0;
mutex_lock(&kernfs_mutex);
down_read(&kernfs_rwsem);
}
mutex_unlock(&kernfs_mutex);
up_read(&kernfs_rwsem);
file->private_data = NULL;
ctx->pos = INT_MAX;
return 0;
Expand Down
4 changes: 2 additions & 2 deletions fs/kernfs/file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ static void kernfs_notify_workfn(struct work_struct *work)
spin_unlock_irq(&kernfs_notify_lock);

/* kick fsnotify */
mutex_lock(&kernfs_mutex);
down_write(&kernfs_rwsem);

list_for_each_entry(info, &kernfs_root(kn)->supers, node) {
struct kernfs_node *parent;
Expand Down Expand Up @@ -898,7 +898,7 @@ static void kernfs_notify_workfn(struct work_struct *work)
iput(inode);
}

mutex_unlock(&kernfs_mutex);
up_write(&kernfs_rwsem);
kernfs_put(kn);
goto repeat;
}
Expand Down
Loading

0 comments on commit 7ba0273

Please sign in to comment.