Date: Wed, 20 May 2009 09:38:13 +0100 From: Doug Rabson <dfr@rabson.org> To: Doug Rabson <dfr@rabson.org> Cc: svn-src-head@freebsd.org, Rick Macklem <rmacklem@FreeBSD.org>, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r192256 - head/sys/fs/nfsserver Message-ID: <E45903EF-81D4-482D-88B1-B763440D8962@rabson.org> In-Reply-To: <8ECF61A0-AFE1-4320-B0AA-2216C268A921@rabson.org> References: <200905171933.n4HJXmC0037587@svn.freebsd.org> <8ECF61A0-AFE1-4320-B0AA-2216C268A921@rabson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Thinking about this for a few more minutes, I think you probably want to allocate a sysid for each client and then for each lock_owner of that client allocate a 'pid'. The value doesn't have to be a process identifier but it does have to allow different lock owners from the same client to be distinguished. You probably also want to record locks in the local lock manager on the client. In NLM, I use a different range of sysids starting at 0x100000 for this. This lets you do lock recovery after a server restart by asking the local lock manager to enumerate locks for the right sysid. On 20 May 2009, at 09:26, Doug Rabson wrote: > This is incorrect. A sysid of zero is reserved for local locks on > local filesystems. You need to allocate a sysid when the client is > created and it needs to not conflict with the sysids used by NLM. I > suggest adding a function to nlm_prot_impl.c to return the next > available sysid (and bump the counter). > > On 17 May 2009, at 20:33, Rick Macklem wrote: > >> Author: rmacklem >> Date: Sun May 17 19:33:48 2009 >> New Revision: 192256 >> URL: http://svn.freebsd.org/changeset/base/192256 >> >> Log: >> Fix the acquisition of local locks via VOP_ADVLOCK() by the >> experimental nfsv4 server. It was setting the a_id argument >> to a fixed value, but that wasn't sufficient for FreeBSD8. >> Instead, set l_pid and l_sysid to 0 plus set the F_REMOTE >> flag to indicate that these fields are used to check for >> same lock owner. Since, for NFSv4, a lockowner is a ClientID plus >> an up to 1024byte name, it can't be put in l_sysid easily. >> I also renamed the p variable to td, since it's a thread ptr. >> >> Approved by: kib (mentor) >> >> Modified: >> head/sys/fs/nfsserver/nfs_nfsdport.c >> >> Modified: head/sys/fs/nfsserver/nfs_nfsdport.c >> = >> = >> = >> = >> = >> = >> = >> = >> = >> ===================================================================== >> --- head/sys/fs/nfsserver/nfs_nfsdport.c Sun May 17 17:54:01 2009 >> (r192255) >> +++ head/sys/fs/nfsserver/nfs_nfsdport.c Sun May 17 19:33:48 2009 >> (r192256) >> @@ -2749,14 +2749,13 @@ nfsvno_getvp(fhandle_t *fhp) >> return (vp); >> } >> >> -static int id_for_advlock; >> /* >> * Check to see it a byte range lock held by a process running >> * locally on the server conflicts with the new lock. >> */ >> int >> nfsvno_localconflict(struct vnode *vp, int ftype, u_int64_t first, >> - u_int64_t end, struct nfslockconflict *cfp, struct thread *p) >> + u_int64_t end, struct nfslockconflict *cfp, struct thread *td) >> { >> int error; >> struct flock fl; >> @@ -2771,11 +2770,20 @@ nfsvno_localconflict(struct vnode *vp, i >> else >> fl.l_len = (off_t)(end - first); >> /* >> - * FreeBSD8 doesn't like 0, so I'll use the address of >> id_for_advlock. >> + * For FreeBSD8, the l_pid and l_sysid must be set to the same >> + * values for all calls, so that all locks will be held by the >> + * nfsd server. (The nfsd server handles conflicts between the >> + * various clients.) >> + * Since an NFSv4 lockowner is a ClientID plus an array of up to >> 1024 >> + * bytes, so it can't be put in l_sysid. >> */ >> - NFSVOPUNLOCK(vp, 0, p); >> - error = VOP_ADVLOCK(vp, &id_for_advlock, F_GETLK, &fl, F_POSIX); >> - NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, p); >> + fl.l_pid = (pid_t)0; >> + fl.l_sysid = 0; >> + >> + NFSVOPUNLOCK(vp, 0, td); >> + error = VOP_ADVLOCK(vp, (caddr_t)td->td_proc, F_GETLK, &fl, >> + (F_POSIX | F_REMOTE)); >> + NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, td); >> if (error) >> return (error); >> if (fl.l_type == F_UNLCK) >> @@ -2804,7 +2812,7 @@ nfsvno_localconflict(struct vnode *vp, i >> */ >> int >> nfsvno_advlock(struct vnode *vp, int ftype, u_int64_t first, >> - u_int64_t end, struct thread *p) >> + u_int64_t end, struct thread *td) >> { >> int error; >> struct flock fl; >> @@ -2822,11 +2830,20 @@ nfsvno_advlock(struct vnode *vp, int fty >> fl.l_len = (off_t)tlen; >> } >> /* >> - * FreeBSD8 doesn't like 0, so I'll use the address of >> id_for_advlock. >> + * For FreeBSD8, the l_pid and l_sysid must be set to the same >> + * values for all calls, so that all locks will be held by the >> + * nfsd server. (The nfsd server handles conflicts between the >> + * various clients.) >> + * Since an NFSv4 lockowner is a ClientID plus an array of up to >> 1024 >> + * bytes, so it can't be put in l_sysid. >> */ >> - NFSVOPUNLOCK(vp, 0, p); >> - error = VOP_ADVLOCK(vp, &id_for_advlock, F_SETLK, &fl, F_POSIX); >> - NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, p); >> + fl.l_pid = (pid_t)0; >> + fl.l_sysid = 0; >> + >> + NFSVOPUNLOCK(vp, 0, td); >> + error = VOP_ADVLOCK(vp, (caddr_t)td->td_proc, F_SETLK, &fl, >> + (F_POSIX | F_REMOTE)); >> + NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, td); >> return (error); >> } >> >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E45903EF-81D4-482D-88B1-B763440D8962>