Date: Wed, 20 May 2009 09:26:51 +0100 From: Doug Rabson <dfr@rabson.org> To: Rick Macklem <rmacklem@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r192256 - head/sys/fs/nfsserver Message-ID: <8ECF61A0-AFE1-4320-B0AA-2216C268A921@rabson.org> In-Reply-To: <200905171933.n4HJXmC0037587@svn.freebsd.org> References: <200905171933.n4HJXmC0037587@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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?8ECF61A0-AFE1-4320-B0AA-2216C268A921>