Date: Sat, 28 Aug 2010 21:41:18 +0000 (UTC) From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r211951 - in head/sys/fs: nfs nfsserver Message-ID: <201008282141.o7SLfITb044206@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rmacklem Date: Sat Aug 28 21:41:18 2010 New Revision: 211951 URL: http://svn.freebsd.org/changeset/base/211951 Log: 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 Modified: head/sys/fs/nfs/nfs_commonsubs.c head/sys/fs/nfs/nfs_var.h head/sys/fs/nfsserver/nfs_nfsdsocket.c head/sys/fs/nfsserver/nfs_nfsdstate.c Modified: head/sys/fs/nfs/nfs_commonsubs.c ============================================================================== --- head/sys/fs/nfs/nfs_commonsubs.c Sat Aug 28 21:15:00 2010 (r211950) +++ head/sys/fs/nfs/nfs_commonsubs.c Sat Aug 28 21:41:18 2010 (r211951) @@ -1824,6 +1824,21 @@ nfsv4_getref(struct nfsv4lock *lp, int * } /* + * 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 Modified: head/sys/fs/nfs/nfs_var.h ============================================================================== --- head/sys/fs/nfs/nfs_var.h Sat Aug 28 21:15:00 2010 (r211950) +++ head/sys/fs/nfs/nfs_var.h Sat Aug 28 21:41:18 2010 (r211951) @@ -251,6 +251,7 @@ int nfsv4_lock(struct nfsv4lock *, int, 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); Modified: head/sys/fs/nfsserver/nfs_nfsdsocket.c ============================================================================== --- head/sys/fs/nfsserver/nfs_nfsdsocket.c Sat Aug 28 21:15:00 2010 (r211950) +++ head/sys/fs/nfsserver/nfs_nfsdsocket.c Sat Aug 28 21:41:18 2010 (r211951) @@ -533,8 +533,6 @@ nfsrvd_compound(struct nfsrv_descript *n 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 Modified: head/sys/fs/nfsserver/nfs_nfsdstate.c ============================================================================== --- head/sys/fs/nfsserver/nfs_nfsdstate.c Sat Aug 28 21:15:00 2010 (r211950) +++ head/sys/fs/nfsserver/nfs_nfsdstate.c Sat Aug 28 21:41:18 2010 (r211951) @@ -164,8 +164,6 @@ nfsrv_setclient(struct nfsrv_descript *n 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 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 *revo 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_ /* * 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 *c 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 NFSV4ROOTLOCKMUTEXPTR); } while (!gotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ *haslockp = 1; NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, p); return (-1);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201008282141.o7SLfITb044206>