Subject: Re: [nfsv4] LOCK and locker

From: "J. Bruce Fields" <[email-masked]>
Date: Fri, 4 Nov 2011 17:15:09 -0400

On Thu, Jun 30, 2011 at 12:04:33PM -0400, Trond Myklebust wrote:
> On Thu, 2011-06-30 at 11:53 -0400, J. Bruce Fields wrote: 
> > On Thu, Jun 30, 2011 at 11:37:58AM -0400, Trond Myklebust wrote:
> > > On Thu, 2011-06-30 at 11:05 -0400, J. Bruce Fields wrote: 
> > > > ... and ditto for this case; the session ordering means a server won't
> > > > see two such open_to_lockowner's without a client actually sending them,
> > > > but catching this case on the server side could be useful as a debugging
> > > > aid.
> > > 
> > > Err... Where is the text that says the client has to send all LOCK/LOCKU
> > > requests on the same slot?
> > 
> > Agreed.  What the session does for us here is tells the server that if
> > it gets two open_to_lockowner's for the same (file, lockowner string)
> > pair, then that must be because the client actually sent both of them.
> 
> Agreed.
> 
> > > There is no session ordering, and the client isn't buggy if it sends
> > > LOCK and LOCKU on different slots.
> > 
> > So this ambiguous lock ordering will occur if a client actually chooses
> > it by sending two such open_to_lockowners.
> 
> Why? Where does the spec allow you to do this?
> 
> > Are we in violent agreement here, or am I missing something else?  Does
> > a server catching this case do anything more than catch buggy clients,
> > or are there legitimate reasons why a client would do two
> > open_to_lockowners for the same (file, lockowner string) ?
> 
> If I have two threads that are trying to grab byte range locks on the
> same file (but for different ranges) then why should I be forced to
> order them?
> I don't have to do that for the OPEN calls, or LAYOUTGET calls so why
> are LOCKs such a special case?

Just getting back to this: OK, I agree, I think I was partly confused
about the 4.0/4.1 difference.  So after rereading the LOCK sections in
the two specs, I think the right thing is:

	- return BAD_SEQID in the 4.0 case
	- use any preexisting stateid in the sessions case.

And I have patches in testing.

--b.