On Fri, 9 Jan 2015 09:28:35 -0800 Jeff Layton <jlayton@xxxxxxxxxxxxxxx> wrote: > On Fri, 9 Jan 2015 18:16:41 +0100 > Christoph Hellwig <hch@xxxxxx> wrote: > > > On Fri, Jan 09, 2015 at 08:51:30AM -0800, Jeff Layton wrote: > > > 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). > > > > What kinds of comments do you expect? Not implementing unused features > > of a protocol should be the default for anything in Linux. > > > > I was thinking just a comment saying that ROC is always true in this > implementation, or maybe consider eliminating the lg_roc field in > struct nfsd4_layoutget altogether since it's currently always "1". > > It's a little confusing now since the encoder can handle the case where > lg_roc is 0, but the rest of the code can't. > > > > 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. > > > > It would be good to look at the errata. While the idea of keeping > > layouts around longer makes sense, I would only expect to do this > > if they layout state was created based on a delegation stateid, not > > a lock or open stateid. In that case having the layouts hang off > > the "parent" stateid might be another option. > > I found it: > > http://www.rfc-editor.org/errata_search.php?rfc=5661&eid=3226 > Oh, hmm...except that doesn't seem to have been updated according to the discussion from around a year ago. See the thread entitled: [nfsv4] NFSv4.1 errata id 3226 (the return of return-on-close layouts) ...on thee nfsv4@xxxxxxxx mailing list. Trond mentions it there. Perhaps we need to revise that errata? -- Jeff Layton <jlayton@xxxxxxxxxxxxxxx> -- 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
