Re: [PATCH 09/18] nfsd: implement pNFS operations

[Jeff Layton <jeff.layton@primarydata.com>]

 On Fri, 9 Jan 2015 18:16:41 +0100
Christoph Hellwig <hch@lst.de> 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

-- 
Jeff Layton <jlayton@primarydata.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs