svn commit: r192256 - head/sys/fs/nfsserver

[Doug Rabson]

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);
>> }
>>
>