On Apr. 24, 2010, 3:10 +0300, "J. Bruce Fields" <bfields@xxxxxxxxxxxxxx> wrote: > 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. True. My patchset sort of did that on SEQUENCE, though not under the sessionid_lock. If the client is expired atomically with updating all sessions referring to it under that lock we can refcount it for renewal on nfsd4_get_session (always called under sessionid_lock), next nfsd4_put_session should be moved under the same lock and there the "use count" can be decremented and the client can be renewed when no session is actively using it. Benny > > --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
