2000-10-31 16:15:09

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH] fix deadlocks + blocking in 2.4.0 pre6/7 knfsd locking...


fs/locks.c

- Drop semaphore when we call fl_notify hooks. This is to allow the
notification routines to call posix_unblock_lock().
- In locks_wake_up_blocks(), drop semaphore when we're asking the
waiter waiter to unblock, and reorder loop to protect against the
waiter disappearing beneath us while we're not holding the
semaphore.
- locks_delete_lock() should not call file->f_op->lock(). The
latter notifies the *server*, hence calling it from routines like
posix_lock_file() while merging/unmerging locks in our internal
representation of the locks is a bug.
A new function locks_unlock_delete() takes care of the special
case when we're freeing all locks upon close() and drops the
semaphore before notifying the server.

fs/lockd/clntproc.c

- Grab kernel_lock() when accessing lockd-specific structures from
the higher-level locking via fl_notify/fl_insert/fl_delete.
- We must use locks_copy_lock() in order to copy the actual struct
file_lock in nlmclnt_setgrantargs() if we want to avoid clobbering
the list structure.

fs/lockd/svclock.c
- Fix timeout mechanism in 'nlm_blocked' list:
1) allow for jiffy wraparound - use time_before/after
2) fix NLM_NEVER 'magic value' for no-timeout.
- When removing from the nlm_blocked, ensure that block->b_queued
flag is reset.
- Fix race in block notification with wake up lockd.

net/sunrpc/svcsock.c
- ensure that svc_wake_up() does result in the lockd() routine
being called back, rather than just looping in svc_recv().

Trond


diff -u --recursive --new-file linux-2.4.0-test10-pre6/fs/lockd/clntproc.c linux-2.4.0-test10-fix_locks/fs/lockd/clntproc.c
--- linux-2.4.0-test10-pre6/fs/lockd/clntproc.c Thu Sep 21 22:38:58 2000
+++ linux-2.4.0-test10-fix_locks/fs/lockd/clntproc.c Sat Oct 28 18:25:59 2000
@@ -11,6 +11,7 @@
#include <linux/fs.h>
#include <linux/nfs_fs.h>
#include <linux/utsname.h>
+#include <linux/smp_lock.h>
#include <linux/sunrpc/clnt.h>
#include <linux/sunrpc/svc.h>
#include <linux/lockd/lockd.h>
@@ -64,11 +65,11 @@
int
nlmclnt_setgrantargs(struct nlm_rqst *call, struct nlm_lock *lock)
{
- nlmclnt_next_cookie(&call->a_args.cookie);
- call->a_args.lock = *lock;
+ locks_copy_lock(&call->a_args.lock.fl, &lock->fl);
+ memcpy(&call->a_args.lock.fh, &lock->fh, sizeof(call->a_args.lock.fh));
call->a_args.lock.caller = system_utsname.nodename;
+ call->a_args.lock.oh.len = lock->oh.len;

- init_waitqueue_head(&call->a_args.lock.fl.fl_wait);
/* set default data area */
call->a_args.lock.oh.data = call->a_owner;

@@ -406,15 +407,19 @@
static
void nlmclnt_insert_lock_callback(struct file_lock *fl)
{
+ lock_kernel();
nlm_get_host(fl->fl_u.nfs_fl.host);
+ unlock_kernel();
}
static
void nlmclnt_remove_lock_callback(struct file_lock *fl)
{
+ lock_kernel();
if (fl->fl_u.nfs_fl.host) {
nlm_release_host(fl->fl_u.nfs_fl.host);
fl->fl_u.nfs_fl.host = NULL;
}
+ unlock_kernel();
}

/*
diff -u --recursive --new-file linux-2.4.0-test10-pre6/fs/lockd/svclock.c linux-2.4.0-test10-fix_locks/fs/lockd/svclock.c
--- linux-2.4.0-test10-pre6/fs/lockd/svclock.c Fri Oct 27 14:50:13 2000
+++ linux-2.4.0-test10-fix_locks/fs/lockd/svclock.c Sat Oct 28 18:25:44 2000
@@ -24,6 +24,7 @@
#include <linux/types.h>
#include <linux/errno.h>
#include <linux/kernel.h>
+#include <linux/smp_lock.h>
#include <linux/sunrpc/clnt.h>
#include <linux/sunrpc/svc.h>
#include <linux/lockd/nlm.h>
@@ -53,9 +54,15 @@
dprintk("lockd: nlmsvc_insert_block(%p, %ld)\n", block, when);
if (block->b_queued)
nlmsvc_remove_block(block);
- for (bp = &nlm_blocked; (b = *bp); bp = &b->b_next)
- if (when < b->b_when)
- break;
+ bp = &nlm_blocked;
+ if (when != NLM_NEVER) {
+ if ((when += jiffies) == NLM_NEVER)
+ when ++;
+ while ((b = *bp) && time_before_eq(b->b_when,when))
+ bp = &b->b_next;
+ } else
+ while ((b = *bp))
+ bp = &b->b_next;

block->b_queued = 1;
block->b_when = when;
@@ -106,8 +113,10 @@
(long long)fl->fl_end, fl->fl_type,
*(unsigned int*)(block->b_call.a_args.cookie.data));
if (block->b_file == file && nlm_compare_locks(fl, &lock->fl)) {
- if (remove)
+ if (remove) {
*head = block->b_next;
+ block->b_queued = 0;
+ }
return block;
}
}
@@ -173,10 +182,11 @@
locks_init_lock(&block->b_call.a_args.lock.fl);
locks_init_lock(&block->b_call.a_res.lock.fl);

- /* Set notifier function for VFS, and init args */
- lock->fl.fl_notify = nlmsvc_notify_blocked;
if (!nlmclnt_setgrantargs(&block->b_call, lock))
goto failed_free;
+
+ /* Set notifier function for VFS, and init args */
+ block->b_call.a_args.lock.fl.fl_notify = nlmsvc_notify_blocked;
block->b_call.a_args.cookie = *cookie; /* see above */

dprintk("lockd: created block %p...\n", block);
@@ -457,13 +467,15 @@

dprintk("lockd: VFS unblock notification for block %p\n", fl);
posix_unblock_lock(fl);
+ lock_kernel();
for (bp = &nlm_blocked; (block = *bp); bp = &block->b_next) {
if (nlm_compare_locks(&block->b_call.a_args.lock.fl, fl)) {
- svc_wake_up(block->b_daemon);
nlmsvc_insert_block(block, 0);
+ svc_wake_up(block->b_daemon);
return;
}
}
+ unlock_kernel();

printk(KERN_WARNING "lockd: notification for unknown block!\n");
}
@@ -520,7 +532,7 @@
if ((error = posix_lock_file(&file->f_file, &lock->fl, 0)) < 0) {
printk(KERN_WARNING "lockd: unexpected error %d in %s!\n",
-error, __FUNCTION__);
- nlmsvc_insert_block(block, jiffies + 10 * HZ);
+ nlmsvc_insert_block(block, 10 * HZ);
up(&file->f_sema);
return;
}
@@ -532,7 +544,7 @@
block->b_incall = 1;

/* Schedule next grant callback in 30 seconds */
- nlmsvc_insert_block(block, jiffies + 30 * HZ);
+ nlmsvc_insert_block(block, 30 * HZ);

/* Call the client */
nlm_get_host(block->b_call.a_host);
@@ -570,13 +582,13 @@
* can be done, though. */
if (task->tk_status < 0) {
/* RPC error: Re-insert for retransmission */
- timeout = jiffies + 10 * HZ;
+ timeout = 10 * HZ;
} else if (block->b_done) {
/* Block already removed, kill it for real */
timeout = 0;
} else {
/* Call was successful, now wait for client callback */
- timeout = jiffies + 60 * HZ;
+ timeout = 60 * HZ;
}
nlmsvc_insert_block(block, timeout);
svc_wake_up(block->b_daemon);
@@ -604,7 +616,7 @@
if ((block = nlmsvc_find_block(cookie)) != NULL) {
if (status == NLM_LCK_DENIED_GRACE_PERIOD) {
/* Try again in a couple of seconds */
- nlmsvc_insert_block(block, jiffies + 10 * HZ);
+ nlmsvc_insert_block(block, 10 * HZ);
block = NULL;
} else {
/* Lock is now held by client, or has been rejected.
@@ -635,7 +647,11 @@
dprintk("nlmsvc_retry_blocked(%p, when=%ld)\n",
nlm_blocked,
nlm_blocked? nlm_blocked->b_when : 0);
- while ((block = nlm_blocked) && block->b_when <= jiffies) {
+ while ((block = nlm_blocked)) {
+ if (block->b_when == NLM_NEVER)
+ break;
+ if (time_after(block->b_when,jiffies))
+ break;
dprintk("nlmsvc_retry_blocked(%p, when=%ld, done=%d)\n",
block, block->b_when, block->b_done);
if (block->b_done)
diff -u --recursive --new-file linux-2.4.0-test10-pre6/fs/locks.c linux-2.4.0-test10-fix_locks/fs/locks.c
--- linux-2.4.0-test10-pre6/fs/locks.c Fri Oct 27 14:50:13 2000
+++ linux-2.4.0-test10-fix_locks/fs/locks.c Sat Oct 28 18:12:20 2000
@@ -436,21 +436,28 @@
{
while (!list_empty(&blocker->fl_block)) {
struct file_lock *waiter = list_entry(blocker->fl_block.next, struct file_lock, fl_block);
+
+ if (!wait) {
+ /* Remove waiter from the block list, because by the
+ * time it wakes up blocker won't exist any more.
+ */
+ locks_delete_block(waiter);
+ }
/* N.B. Is it possible for the notify function to block?? */
- if (waiter->fl_notify)
+ if (waiter->fl_notify) {
+ release_fl_sem();
waiter->fl_notify(waiter);
- wake_up(&waiter->fl_wait);
+ acquire_fl_sem();
+ } else
+ wake_up(&waiter->fl_wait);
if (wait) {
+ release_fl_sem();
/* Let the blocked process remove waiter from the
* block list when it gets scheduled.
*/
current->policy |= SCHED_YIELD;
schedule();
- } else {
- /* Remove waiter from the block list, because by the
- * time it wakes up blocker won't exist any more.
- */
- locks_delete_block(waiter);
+ acquire_fl_sem();
}
}
}
@@ -477,7 +484,6 @@
*/
static void locks_delete_lock(struct file_lock **thisfl_p, unsigned int wait)
{
- int (*lock)(struct file *, int, struct file_lock *);
struct file_lock *fl = *thisfl_p;

*thisfl_p = fl->fl_next;
@@ -496,12 +502,26 @@
fl->fl_remove(fl);

locks_wake_up_blocks(fl, wait);
- lock = fl->fl_file->f_op->lock;
- if (lock) {
+ locks_free_lock(fl);
+}
+
+/*
+ * Call back client filesystem in order to get it to unregister a lock,
+ * then delete lock. Essentially useful only in locks_remove_*().
+ * Note: this must be called with the semaphore already held!
+ */
+static inline void locks_unlock_delete(struct file_lock **thisfl_p)
+{
+ struct file_lock *fl = *thisfl_p;
+ int (*lock)(struct file *, int, struct file_lock *);
+
+ if ((lock = fl->fl_file->f_op->lock) != NULL) {
fl->fl_type = F_UNLCK;
+ release_fl_sem();
lock(fl->fl_file, F_SETLK, fl);
+ acquire_fl_sem();
}
- locks_free_lock(fl);
+ locks_delete_lock(thisfl_p, 0);
}

/* Determine if lock sys_fl blocks lock caller_fl. Common functionality
@@ -1644,11 +1664,12 @@
return;
}
acquire_fl_sem();
+ again:
before = &inode->i_flock;
while ((fl = *before) != NULL) {
if ((fl->fl_flags & FL_POSIX) && fl->fl_owner == owner) {
- locks_delete_lock(before, 0);
- continue;
+ locks_unlock_delete(before);
+ goto again;
}
before = &fl->fl_next;
}
diff -u --recursive --new-file linux-2.4.0-test10-pre6/net/sunrpc/svcsock.c linux-2.4.0-test10-fix_locks/net/sunrpc/svcsock.c
--- linux-2.4.0-test10-pre6/net/sunrpc/svcsock.c Sat Jul 8 00:57:49 2000
+++ linux-2.4.0-test10-fix_locks/net/sunrpc/svcsock.c Fri Oct 27 17:12:11 2000
@@ -800,7 +800,6 @@
"svc_recv: service %p, wait queue active!\n",
rqstp);

-again:
/* Initialize the buffers */
rqstp->rq_argbuf = rqstp->rq_defbuf;
rqstp->rq_resbuf = rqstp->rq_defbuf;
@@ -846,7 +845,7 @@
/* No data, incomplete (TCP) read, or accept() */
if (len == 0 || len == -EAGAIN) {
svc_sock_release(rqstp);
- goto again;
+ return -EAGAIN;
}

rqstp->rq_secure = ntohs(rqstp->rq_addr.sin_port) < 1024;