On Mon, Jan 14, 2008 at 09:28:30PM -0700, Matthew Wilcox wrote:
>
> interruptible_sleep_on_locked() is just an open-coded
> wait_event_interruptible_timeout() with a few assumptions since we know
> we hold the BKL. locks_block_on_timeout() is only used in one place, so
> it's actually simpler to inline it into its caller.
Makes sense, thanks. So the assumption we were depending on the BKL for
was that we could count on the wake-up not coming till after we block,
so we could skip a check ->fl_next that's normally needed to resolve the
usual sleeping-on-some-condition race?
--b.
>
> Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxxxx>
>
> locks.c | 33 ++++-----------------------------
> 1 file changed, 4 insertions(+), 29 deletions(-)
>
> diff --git a/fs/locks.c b/fs/locks.c
> index 8b8388e..b681459 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -634,33 +634,6 @@ static int flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *s
> return (locks_conflict(caller_fl, sys_fl));
> }
>
> -static int interruptible_sleep_on_locked(wait_queue_head_t *fl_wait, int timeout)
> -{
> - int result = 0;
> - DECLARE_WAITQUEUE(wait, current);
> -
> - __set_current_state(TASK_INTERRUPTIBLE);
> - add_wait_queue(fl_wait, &wait);
> - if (timeout == 0)
> - schedule();
> - else
> - result = schedule_timeout(timeout);
> - if (signal_pending(current))
> - result = -ERESTARTSYS;
> - remove_wait_queue(fl_wait, &wait);
> - __set_current_state(TASK_RUNNING);
> - return result;
> -}
> -
> -static int locks_block_on_timeout(struct file_lock *blocker, struct file_lock *waiter, int time)
> -{
> - int result;
> - locks_insert_block(blocker, waiter);
> - result = interruptible_sleep_on_locked(&waiter->fl_wait, time);
> - __locks_delete_block(waiter);
> - return result;
> -}
> -
> void
> posix_test_lock(struct file *filp, struct file_lock *fl)
> {
> @@ -1256,7 +1229,10 @@ restart:
> if (break_time == 0)
> break_time++;
> }
> - error = locks_block_on_timeout(flock, new_fl, break_time);
> + locks_insert_block(flock, new_fl);
> + error = wait_event_interruptible_timeout(new_fl->fl_wait,
> + !new_fl->fl_next, break_time);
> + __locks_delete_block(new_fl);
> if (error >= 0) {
> if (error == 0)
> time_out_leases(inode);
>
> --
> Intel are signing my paycheques ... these opinions are still mine
> "Bill, look, we understand that you're interested in selling us this
> operating system, but compare it to ours. We can't possibly take such
> a retrograde step."
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
