On Fri, 9 Jan 2015 11:05:51 +0100 Christoph Hellwig <hch@xxxxxx> wrote: > On Thu, Jan 08, 2015 at 04:48:51PM -0800, Jeff Layton wrote: > > > + nfsd4_return_all_file_layouts(stp->st_stateowner->so_client, > > > + stp->st_stid.sc_file); > > > + > > > > Shouldn't the above be conditional on whether the lg_roc was true? > > There is no support for non-lg_roc layouts at the moment. > > In general I've avoided adding code that isn't used, as they can't > be tested and thus most likely won't work. Ok, it'd be good to document that in some comments then for the sake of posterity (maybe it is later in the set -- I haven't gotten to the end yet). Now, that said...I think that your ROC semantics are wrong here. You also have to take delegations into account. [1] Basically the semantics that you want are that nfsd should do all of the ROC stuff on last close iff there are no outstanding delegations or on delegreturn iff there are no opens. What we ended up doing in the unreleased code we have was to create a new per-client and per-file object (that we creatively called an "odstate"). An open stateid and a delegation stateid would hold a reference to this object which is put when those stateids are freed. When its refcount goes to zero, then we'd free any outstanding layouts on the file for that client and free the object. You probably want to do something similar here. [1]: Tom and Trond mentioned that there's a RFC5661 errata pending for this, but I don't see it right offhand. -- Jeff Layton <jlayton@xxxxxxxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html