diff options
author | rmacklem <rmacklem@FreeBSD.org> | 2010-08-28 21:41:18 +0000 |
---|---|---|
committer | rmacklem <rmacklem@FreeBSD.org> | 2010-08-28 21:41:18 +0000 |
commit | 3b7498e206fce85bc672ab8f554bcd6ebc0b398f (patch) | |
tree | a22b84f7bb4316f13607b78f621510c2215914fc /sys/fs | |
parent | 10773ba533b6afc49c6a607ff7b4c8dd6a3e0950 (diff) | |
download | FreeBSD-src-3b7498e206fce85bc672ab8f554bcd6ebc0b398f.zip FreeBSD-src-3b7498e206fce85bc672ab8f554bcd6ebc0b398f.tar.gz |
The timer routine in the experimental NFS server did not acquire
the correct mutex when checking nfsv4root_lock. Although this
could be fixed by adding mutex lock/unlock calls, zack.kirsch at
isilon.com suggested a better fix that uses a non-blocking
acquisition of a reference count on nfsv4root_lock. This fix
allows the weird NFSLOCKSTATE(); NFSUNLOCKSTATE(); synchronization
to be deleted. This patch applies this fix.
Tested by: zack.kirsch at isilon.com
MFC after: 2 weeks
Diffstat (limited to 'sys/fs')
-rw-r--r-- | sys/fs/nfs/nfs_commonsubs.c | 15 | ||||
-rw-r--r-- | sys/fs/nfs/nfs_var.h | 1 | ||||
-rw-r--r-- | sys/fs/nfsserver/nfs_nfsdsocket.c | 2 | ||||
-rw-r--r-- | sys/fs/nfsserver/nfs_nfsdstate.c | 33 |
4 files changed, 28 insertions, 23 deletions
diff --git a/sys/fs/nfs/nfs_commonsubs.c b/sys/fs/nfs/nfs_commonsubs.c index dc5b2dc..6585fce 100644 --- a/sys/fs/nfs/nfs_commonsubs.c +++ b/sys/fs/nfs/nfs_commonsubs.c @@ -1824,6 +1824,21 @@ nfsv4_getref(struct nfsv4lock *lp, int *isleptp, void *mutex) } /* + * Get a reference as above, but return failure instead of sleeping if + * an exclusive lock is held. + */ +APPLESTATIC int +nfsv4_getref_nonblock(struct nfsv4lock *lp) +{ + + if ((lp->nfslock_lock & NFSV4LOCK_LOCK) != 0) + return (0); + + lp->nfslock_usecnt++; + return (1); +} + +/* * Test for a lock. Return 1 if locked, 0 otherwise. */ APPLESTATIC int diff --git a/sys/fs/nfs/nfs_var.h b/sys/fs/nfs/nfs_var.h index d6ecda2..d46983f 100644 --- a/sys/fs/nfs/nfs_var.h +++ b/sys/fs/nfs/nfs_var.h @@ -251,6 +251,7 @@ int nfsv4_lock(struct nfsv4lock *, int, int *, void *); void nfsv4_unlock(struct nfsv4lock *, int); void nfsv4_relref(struct nfsv4lock *); void nfsv4_getref(struct nfsv4lock *, int *, void *); +int nfsv4_getref_nonblock(struct nfsv4lock *); int nfsv4_testlock(struct nfsv4lock *); int nfsrv_mtostr(struct nfsrv_descript *, char *, int); int nfsrv_checkutf8(u_int8_t *, int); diff --git a/sys/fs/nfsserver/nfs_nfsdsocket.c b/sys/fs/nfsserver/nfs_nfsdsocket.c index c95fef2..af1a3fa 100644 --- a/sys/fs/nfsserver/nfs_nfsdsocket.c +++ b/sys/fs/nfsserver/nfs_nfsdsocket.c @@ -533,8 +533,6 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, NFSV4ROOTLOCKMUTEXPTR); NFSUNLOCKV4ROOTMUTEX(); if (igotlock) { - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ /* * If I got the lock, I can update the stable storage file. * Done when the grace period is over or a client has long diff --git a/sys/fs/nfsserver/nfs_nfsdstate.c b/sys/fs/nfsserver/nfs_nfsdstate.c index 6c89e6b..c96907e 100644 --- a/sys/fs/nfsserver/nfs_nfsdstate.c +++ b/sys/fs/nfsserver/nfs_nfsdstate.c @@ -164,8 +164,6 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp, NFSV4ROOTLOCKMUTEXPTR); } while (!igotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ /* * Search for a match in the client list. @@ -416,8 +414,6 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp, NFSV4ROOTLOCKMUTEXPTR); } while (!igotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ } else if (opflags != CLOPS_RENEW) { NFSLOCKSTATE(); } @@ -547,8 +543,6 @@ nfsrv_adminrevoke(struct nfsd_clid *revokep, NFSPROC_T *p) NFSV4ROOTLOCKMUTEXPTR); } while (!igotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ /* * Search for a match in the client list. @@ -824,11 +818,8 @@ nfsrv_dumplocks(vnode_t vp, struct nfsd_dumplocks *ldumpp, int maxcnt, /* * Server timer routine. It can scan any linked list, so long - * as it holds the spin lock and there is no exclusive lock on + * as it holds the spin/mutex lock and there is no exclusive lock on * nfsv4rootfs_lock. - * Must be called by a kernel thread and not a timer interrupt, - * so that it only runs when the nfsd threads are sleeping on a - * uniprocessor and uses the State spin lock for an SMP system. * (For OpenBSD, a kthread is ok. For FreeBSD, I think it is ok * to do this from a callout, since the spin locks work. For * Darwin, I'm not sure what will work correctly yet.) @@ -839,7 +830,7 @@ nfsrv_servertimer(void) { struct nfsclient *clp, *nclp; struct nfsstate *stp, *nstp; - int i; + int got_ref, i; /* * Make sure nfsboottime is set. This is used by V3 as well @@ -867,13 +858,14 @@ nfsrv_servertimer(void) } /* - * Return now if an nfsd thread has the exclusive lock on - * nfsv4rootfs_lock. The dirty trick here is that we have - * the spin lock already and the nfsd threads do a: - * NFSLOCKSTATE, NFSUNLOCKSTATE after getting the exclusive - * lock, so they won't race with code after this check. + * Try and get a reference count on the nfsv4rootfs_lock so that + * no nfsd thread can acquire an exclusive lock on it before this + * call is done. If it is already exclusively locked, just return. */ - if (nfsv4rootfs_lock.nfslock_lock & NFSV4LOCK_LOCK) { + NFSLOCKV4ROOTMUTEX(); + got_ref = nfsv4_getref_nonblock(&nfsv4rootfs_lock); + NFSUNLOCKV4ROOTMUTEX(); + if (got_ref == 0) { NFSUNLOCKSTATE(); return; } @@ -945,6 +937,9 @@ nfsrv_servertimer(void) } } NFSUNLOCKSTATE(); + NFSLOCKV4ROOTMUTEX(); + nfsv4_relref(&nfsv4rootfs_lock); + NFSUNLOCKV4ROOTMUTEX(); } /* @@ -4224,8 +4219,6 @@ nfsrv_clientconflict(struct nfsclient *clp, int *haslockp, __unused vnode_t vp, NFSV4ROOTLOCKMUTEXPTR); } while (!gotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ *haslockp = 1; NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, p); return (1); @@ -4390,8 +4383,6 @@ nfsrv_delegconflict(struct nfsstate *stp, int *haslockp, NFSPROC_T *p, NFSV4ROOTLOCKMUTEXPTR); } while (!gotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ *haslockp = 1; NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, p); return (-1); |