Skip to content

Commit

Permalink
fs/locks: allow a lock request to block other requests.
Browse files Browse the repository at this point in the history
Currently, a lock can block pending requests, but all pending
requests are equal.  If lots of pending requests are
mutually exclusive, this means they will all be woken up
and all but one will fail.  This can hurt performance.

So we will allow pending requests to block other requests.
Only the first request will be woken, and it will wake the others.

This patch doesn't implement this fully, but prepares the way.

- It acknowledges that a request might be blocking other requests,
  and when the request is converted to a lock, those blocked
  requests are moved across.
- When a request is requeued or discarded, all blocked requests are
  woken.
- When deadlock-detection looks for the lock which blocks a
  given request, we follow the chain of ->fl_blocker all
  the way to the top.

Tested-by: kernel test robot <rong.a.chen@intel.com>
Signed-off-by: NeilBrown <neilb@suse.com>
Reviewed-by: J. Bruce Fields <bfields@redhat.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
  • Loading branch information
NeilBrown authored and Jeff Layton committed Nov 30, 2018
1 parent d6367d6 commit 5946c43
Showing 1 changed file with 37 additions and 6 deletions.
43 changes: 37 additions & 6 deletions fs/locks.c
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,24 @@ void locks_copy_lock(struct file_lock *new, struct file_lock *fl)

EXPORT_SYMBOL(locks_copy_lock);

static void locks_move_blocks(struct file_lock *new, struct file_lock *fl)
{
struct file_lock *f;

/*
* As ctx->flc_lock is held, new requests cannot be added to
* ->fl_blocked_requests, so we don't need a lock to check if it
* is empty.
*/
if (list_empty(&fl->fl_blocked_requests))
return;
spin_lock(&blocked_lock_lock);
list_splice_init(&fl->fl_blocked_requests, &new->fl_blocked_requests);
list_for_each_entry(f, &fl->fl_blocked_requests, fl_blocked_member)
f->fl_blocker = new;
spin_unlock(&blocked_lock_lock);
}

static inline int flock_translate_cmd(int cmd) {
if (cmd & LOCK_MAND)
return cmd & (LOCK_MAND | LOCK_RW);
Expand Down Expand Up @@ -693,6 +711,7 @@ static void __locks_wake_up_blocks(struct file_lock *blocker)
static void locks_delete_block(struct file_lock *waiter)
{
spin_lock(&blocked_lock_lock);
__locks_wake_up_blocks(waiter);
__locks_delete_block(waiter);
spin_unlock(&blocked_lock_lock);
}
Expand All @@ -716,6 +735,12 @@ static void __locks_insert_block(struct file_lock *blocker,
list_add_tail(&waiter->fl_blocked_member, &blocker->fl_blocked_requests);
if (IS_POSIX(blocker) && !IS_OFDLCK(blocker))
locks_insert_global_blocked(waiter);

/* The requests in waiter->fl_blocked are known to conflict with
* waiter, but might not conflict with blocker, or the requests
* and lock which block it. So they all need to be woken.
*/
__locks_wake_up_blocks(waiter);
}

/* Must be called with flc_lock held. */
Expand Down Expand Up @@ -888,8 +913,11 @@ static struct file_lock *what_owner_is_waiting_for(struct file_lock *block_fl)
struct file_lock *fl;

hash_for_each_possible(blocked_hash, fl, fl_link, posix_owner_key(block_fl)) {
if (posix_same_owner(fl, block_fl))
return fl->fl_blocker;
if (posix_same_owner(fl, block_fl)) {
while (fl->fl_blocker)
fl = fl->fl_blocker;
return fl;
}
}
return NULL;
}
Expand Down Expand Up @@ -982,6 +1010,7 @@ static int flock_lock_inode(struct inode *inode, struct file_lock *request)
if (request->fl_flags & FL_ACCESS)
goto out;
locks_copy_lock(new_fl, request);
locks_move_blocks(new_fl, request);
locks_insert_lock_ctx(new_fl, &ctx->flc_flock);
new_fl = NULL;
error = 0;
Expand Down Expand Up @@ -1175,6 +1204,7 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request,
goto out;
}
locks_copy_lock(new_fl, request);
locks_move_blocks(new_fl, request);
locks_insert_lock_ctx(new_fl, &fl->fl_list);
fl = new_fl;
new_fl = NULL;
Expand Down Expand Up @@ -2586,13 +2616,14 @@ void locks_remove_file(struct file *filp)
int
posix_unblock_lock(struct file_lock *waiter)
{
int status = 0;
int status = -ENOENT;

spin_lock(&blocked_lock_lock);
if (waiter->fl_blocker)
if (waiter->fl_blocker) {
__locks_wake_up_blocks(waiter);
__locks_delete_block(waiter);
else
status = -ENOENT;
status = 0;
}
spin_unlock(&blocked_lock_lock);
return status;
}
Expand Down

0 comments on commit 5946c43

Please sign in to comment.