Received-SPF: pass (google.com: best guess record for domain of linux-nfs-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 2 May 2019 07:35:00 -0400
From:   Scott Mayhew <smayhew@redhat.com>
To:     Trond Myklebust <trondmy@hammerspace.com>
Cc:     "bfields@fieldses.org" <bfields@fieldses.org>,
        "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
        "jlayton@kernel.org" <jlayton@kernel.org>
Subject: Re: [PATCH] nfsd: CB_RECALL can race with FREE_STATEID
Message-ID: <20190502113500.GH15226@coeurl.usersys.redhat.com>
References: <20190418132400.24161-1-smayhew@redhat.com>
 <20190418151312.GB29274@fieldses.org>
 <20190418205024.GB15226@coeurl.usersys.redhat.com>
 <20190418213730.GA1891@fieldses.org>
 <20190430185845.GG15226@coeurl.usersys.redhat.com>
 <5ef90bd5ac79236458ffa04b7eb1d7812431859f.camel@hammerspace.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5ef90bd5ac79236458ffa04b7eb1d7812431859f.camel@hammerspace.com>
User-Agent: Mutt/1.11.3 (2019-02-01)
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Tue, 30 Apr 2019, Trond Myklebust wrote:

> On Tue, 2019-04-30 at 14:58 -0400, Scott Mayhew wrote:
> > On Thu, 18 Apr 2019, J. Bruce Fields wrote:
> > 
> > > On Thu, Apr 18, 2019 at 04:50:24PM -0400, Scott Mayhew wrote:
> > > > On Thu, 18 Apr 2019, J. Bruce Fields wrote:
> > > > 
> > > > > On Thu, Apr 18, 2019 at 09:24:00AM -0400, Scott Mayhew wrote:
> > > > > > While trying to track down some issues involving large
> > > > > > numbers of
> > > > > > delegations being recalled/revoked, I caught the server
> > > > > > setting
> > > > > > SEQ4_STATUS_CB_PATH_DOWN while the client was actively
> > > > > > responding to
> > > > > > CB_RECALLs.  It turns out that the client had already done a
> > > > > > TEST_STATEID and FREE_STATEID for a delegation being recalled
> > > > > > by the
> > > > > > time it received the CB_RECALL.
> > > > > 
> > > > > That's interesting, thanks!
> > > > > 
> > > > > This exception seems awfully narrow, though.
> > > > > 
> > > > > If we get back any NFS-level error at all, then I think the
> > > > > callback
> > > > > channel is working (am I wrong?)
> > > > 
> > > > Correct, if the client replies with either NFS4ERR_DELAY or
> > > > NFS4ERR_BAD_STATEID, the server will retry 1 time (see
> > > > dl_retries).
> > > > After that, we fall thru and nfsd4_cb_recall_done() returns -1
> > > > which
> > > > causes the SEQ4_STATUS_CB_PATH_DOWN flag to be set.
> > > > 
> > > > > and telling the client to set up a new
> > > > > one is probably not going to help.  The best we can do is
> > > > > probably just
> > > > > give up
> > > > 
> > > > That's what the patch is essentially doing.  Or are you saying
> > > > don't
> > > > even bother with the checks but still return 1 so we don't set
> > > > the
> > > > SEQ4_STATUS_CB_PATH_DOWN flag?
> > > 
> > > Right, I don't see any point returning -1 (which ends up setting
> > > CB_PATH_DOWN) in any case where we get an nfs-level error.  If the
> > > client got so far as returning an error, then the callback path is
> > > working.
> > > 
> > > I'm not sure exactly what errors *should* result in CB_PATH_DOWN,
> > > though.  ETIMEDOUT, ENOTCONN, EIO?
> > 
> > I'm not sure either.  Looking at
> > call_status/call_timeout/rpc_check_timeout, it looks to me like
> > ENOTCONN
> > will be translated to ETIMEDOUT because nfsd4_run_cb_work sets the 
> > RPC_TASK_SOFTCONN flag in the call to rpc_call_async.
> > 
> > It looks like call_status can return EHOSTDOWN, ENETDOWN,
> > EHOSTUNREACH,
> > ENETUNREACH, and EPERM... should those be handled as well?
> 
> Those errors should never be passed back to applications.

I'm confused.  If call_status passes any of those errors to rpc_exit,
then I'll see them in rpc_call_done/nfsd4_cb_done, won't I?

-Scott

> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
> 
>