From owner-svn-src-head@FreeBSD.ORG Wed May 20 08:27:22 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EE2AA106564A; Wed, 20 May 2009 08:27:22 +0000 (UTC) (envelope-from dfr@rabson.org) Received: from itchy.rabson.org (router.rabson.org [80.177.232.241]) by mx1.freebsd.org (Postfix) with ESMTP id 697EC8FC13; Wed, 20 May 2009 08:27:22 +0000 (UTC) (envelope-from dfr@rabson.org) Received: from [IPv6:2001:470:909f:1:225:ff:feed:9426] (unknown [IPv6:2001:470:909f:1:225:ff:feed:9426]) by itchy.rabson.org (Postfix) with ESMTP id 127BD5DBC; Wed, 20 May 2009 09:27:56 +0100 (BST) Message-Id: <8ECF61A0-AFE1-4320-B0AA-2216C268A921@rabson.org> From: Doug Rabson To: Rick Macklem In-Reply-To: <200905171933.n4HJXmC0037587@svn.freebsd.org> Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Content-Transfer-Encoding: 7bit Mime-Version: 1.0 (Apple Message framework v935.3) Date: Wed, 20 May 2009 09:26:51 +0100 References: <200905171933.n4HJXmC0037587@svn.freebsd.org> X-Mailer: Apple Mail (2.935.3) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r192256 - head/sys/fs/nfsserver X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 May 2009 08:27:23 -0000 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); > } >