Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <64c4e5c4e61962fd828bcbef79db1df6466a875d.camel@kernel.org>
Subject: Re: [PATCH] SUNRPC: remove the maximum number of retries in
 call_bind_status
From:   Jeff Layton <jlayton@kernel.org>
To:     dai.ngo@oracle.com, Chuck Lever III <chuck.lever@oracle.com>
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Helen Chao <helen.chao@oracle.com>,
        Anna Schumaker <anna@kernel.org>,
        Trond Myklebust <trondmy@hammerspace.com>
Date:   Thu, 06 Apr 2023 13:33:30 -0400
In-Reply-To: <ed870a33-0809-3cfd-2d5a-b97409568b97@oracle.com>
References: <1678301132-24496-1-git-send-email-dai.ngo@oracle.com>
         <9D5A190A-333A-4470-8572-CF85EE9A8086@oracle.com>
         <182842b2-3de4-d64b-d729-f4f6c9c576d6@oracle.com>
         <ed870a33-0809-3cfd-2d5a-b97409568b97@oracle.com>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Tue, 2023-03-14 at 09:19 -0700, dai.ngo@oracle.com wrote:
> On 3/8/23 11:03 AM, dai.ngo@oracle.com wrote:
> > On 3/8/23 10:50 AM, Chuck Lever III wrote:
> > >=20
> > > > On Mar 8, 2023, at 1:45 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
> > > >=20
> > > > Currently call_bind_status places a hard limit of 3 to the number o=
f
> > > > retries on EACCES error. This limit was done to accommodate the=20
> > > > behavior
> > > > of a buggy server that keeps returning garbage when the NLM daemon =
is
> > > > killed on the NFS server. However this change causes problem for ot=
her
> > > > servers that take a little longer than 9 seconds for the port mappe=
r to
> > > > become ready when the NFS server is restarted.
> > > >=20
> > > > This patch removes this hard coded limit and let the RPC handles
> > > > the retry according to whether the export is soft or hard mounted.
> > > >=20
> > > > To avoid the hang with buggy server, the client can use soft mount =
for
> > > > the export.
> > > >=20
> > > > Fixes: 0b760113a3a1 ("NLM: Don't hang forever on NLM unlock request=
s")
> > > > Reported-by: Helen Chao <helen.chao@oracle.com>
> > > > Tested-by: Helen Chao <helen.chao@oracle.com>
> > > > Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> > > Helen is the royal queen of ^C=A0 ;-)
> > >=20
> > > Did you try ^C on a mount while it waits for a rebind?
> >=20
> > She uses a test script that restarts the NFS server while NLM lock test
> > is running. The failure is random, sometimes it fails and sometimes it
> > passes depending on when the LOCK/UNLOCK requests come in so I think
> > it's hard to time it to do the ^C, but I will ask.
>=20
> We did the test with ^C and here is what we found.
>=20
> For synchronous RPC task the signal was delivered to the RPC task and
> the task exit with -ERESTARTSYS from __rpc_execute as expected.
>=20
> For asynchronous RPC task the process that invokes the RPC task to send
> the request detected the signal in rpc_wait_for_completion_task and exits
> with -ERESTARTSYS. However the async RPC was allowed to continue to run
> to completion. So if the async RPC task was retrying an operation and
> the NFS server was down, it will retry forever if this is a hard mount
> or until the NFS server comes back up.
>=20
> The question for the list is should we propagate the signal to the async
> task via rpc_signal_task to stop its execution or just leave it alone as =
is.
>=20
>=20

That is a good question.

I like the patch overall, as it gets rid of a special one-off retry
counter, but I too share some concerns about retrying indefinitely when
an server goes missing.

>=20
Propagating a signal seems like the right thing to do. Looks like
rpcb_getport_done would also need to grow a check for RPC_SIGNALLED ?

It sounds pretty straightforward otherwise.
--=20
Jeff Layton <jlayton@kernel.org>