xfs
[Top] [All Lists]

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

To: Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx>
Subject: Re: [PATCH 09/18] nfsd: implement pNFS operations
From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
Date: Fri, 9 Jan 2015 09:43:41 -0800
Cc: Christoph Hellwig <hch@xxxxxx>, "J. Bruce Fields" <bfields@xxxxxxxxxxxx>, Linux NFS Mailing List <linux-nfs@xxxxxxxxxxxxxxx>, Devel FS Linux <linux-fsdevel@xxxxxxxxxxxxxxx>, xfs@xxxxxxxxxxx, Thomas D Haynes <thomas.haynes@xxxxxxxxxxxxxxx>, Sachin Bhamare <sachin.bhamare@xxxxxxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150109093353.31a2e425@xxxxxxxxxxxxxxxxxxxxxxxxx>
References: <1420561721-9150-1-git-send-email-hch@xxxxxx> <1420561721-9150-10-git-send-email-hch@xxxxxx> <20150108164851.03b64e16@xxxxxxxxxxxxxxxxxxxxxxxxx> <20150109100551.GA23173@xxxxxx> <20150109085130.0f862d24@xxxxxxxxxxxxxxxxxxxxxxxxx> <20150109171641.GA17464@xxxxxx> <20150109092835.5fdbac5d@xxxxxxxxxxxxxxxxxxxxxxxxx> <20150109093353.31a2e425@xxxxxxxxxxxxxxxxxxxxxxxxx>
On Fri, Jan 9, 2015 at 9:33 AM, Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx> wrote:
> 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?
>

Please see:
     http://www.rfc-editor.org/errata_search.php?eid=3901

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>