Re: file locks: Use wait_event_interruptible_timeout()
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 [EMAIL PROTECTED]
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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: file locks: Use wait_event_interruptible_timeout()
On Tue, Jan 15, 2008 at 09:48:51AM -0500, J. Bruce Fields wrote: 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? That's right. -- 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: file locks: Use wait_event_interruptible_timeout()
On Tue, Jan 15, 2008 at 08:04:47AM -0700, Matthew Wilcox wrote: On Tue, Jan 15, 2008 at 09:48:51AM -0500, J. Bruce Fields wrote: 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? That's right. OK, thanks, applied just with the few assumptions replaced by a description of that particular problem: interruptible_sleep_on_locked() is just an open-coded wait_event_interruptible_timeout(), with the one difference that interruptible_sleep_on_locked() doesn't bother to check the condition on which it waits, depending instead on the BKL to avoid the case where it blocks after the wakeup has already been called. locks_block_on_timeout() is only used in one place, so it's actually simpler to inline it into its caller. Pending locks patches available from: git://linux-nfs.org/~bfields/linux.git locks --b. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
file locks: Use wait_event_interruptible_timeout()
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.
Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
