Re: [PATCH 1/2] nfsd4: complete enforcement of 4.1 op ordering

["J. Bruce Fields" <bfields@citi.umich.edu>]

On Fri, Apr 23, 2010 at 07:13:44PM -0400, J. Bruce Fields wrote:
> On Fri, Apr 23, 2010 at 05:24:11PM -0400, J. Bruce Fields wrote:
> > On Thu, Apr 22, 2010 at 10:48:31AM -0400, J. Bruce Fields wrote:
> > > On Thu, Apr 22, 2010 at 10:32:23AM -0400, J. Bruce Fields wrote:
> > > > Enforce the rules about compound op ordering.
> > > > 
> > > > Motivated by implementing RECLAIM_COMPLETE, for which the client is
> > > > implicit in the current session, so it is important to ensure a
> > > > succesful SEQUENCE proceeds the RECLAIM_COMPLETE.
> > > 
> > > The other problem here is that while we have a reference count on the
> > > session itself preventing it from going away till the compound is done,
> > > I don't see what prevents the associated clientid from going away.
> > > 
> > > To fix that, and to be more polite to 4.0 clients, I think we want to
> > > also add a client pointer to the compound_state structure, keep count of
> > > the number of compounds in progress which reference that client, and not
> > > start the client's expiry timer until we've encoded the reply to the
> > > compound.
> > 
> > Benny--I coded up a simple (possibly incorrect) implementation of this,
> > and then remembered that this was more or less what your
> > state-lock-reduction-prep patch series did.  Do you have a more recent
> > version of those patches?
> 
> One possible problem with that approach: mark_for_renew() can fail (if
> the client is already expired), without telling the caller.
> 
> You then end up with an odd situation, continuing to process the rest of
> the compound normally, while your session belongs to a client that's
> already expired.
> 
> Perhaps it would be better to mark the client for possible renewal the
> moment it (or some stateid, sessionid, or other object that refers to
> it) is looked up?

Also: a single "RENEW" state won't suffice when multiple outstanding
compounds reference the same client, in which case you want only the
last (not the first) to complete to do the renewal.  So I think we want
a count of outstanding compounds referencing the client, so that we can
do the renewal when the count goes to zero.

--b.