Received-SPF: pass (google.com: domain of linux-nfs+bounces-3190-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240506210408.4760-1-cel@kernel.org> <CAM5tNy4nceCa7k+E1sKEAi5GYJvMwYDuApspFJvSdVdXBuHjmw@mail.gmail.com>
 <ZjlyT95XGccXXRzL@tissot.1015granger.net>
In-Reply-To: <ZjlyT95XGccXXRzL@tissot.1015granger.net>
From: Rick Macklem <rick.macklem@gmail.com>
Date: Mon, 6 May 2024 17:31:33 -0700
Message-ID: <CAM5tNy6gSqAzWnjyFLynZAPXq1Fa6rjik8x8VwNMLYEgHQm0uA@mail.gmail.com>
Subject: Re: [RFC PATCH] NFSD: Force all NFSv4.2 COPY requests to be synchronous
To: Chuck Lever <chuck.lever@oracle.com>
Cc: cel@kernel.org, linux-nfs@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, May 6, 2024 at 5:14=E2=80=AFPM Chuck Lever <chuck.lever@oracle.com>=
 wrote:
>
> On Mon, May 06, 2024 at 03:30:18PM -0700, Rick Macklem wrote:
> > On Mon, May 6, 2024 at 2:04=E2=80=AFPM <cel@kernel.org> wrote:
> > >
> > > From: Chuck Lever <chuck.lever@oracle.com>
> > >
> > > We've discovered that delivering a CB_OFFLOAD operation can be
> > > unreliable in some pretty unremarkable situations, and the Linux
> > > NFS client does not yet support sending an OFFLOAD_STATUS
> > > operation to probe whether an asynchronous COPY operation has
> > > finished. On Linux NFS clients, COPY can hang until manually
> > > interrupted.
> > >
> > > I've tried a couple of remedies, but so far the side-effects are
> > > worse than the disease. For now, force COPY operations to be
> > > synchronous so that the use of CB_OFFLOAD is avoided entirely.
> > Just a yellow warning light from my experience with the FreeBSD server
> > (which always does synchronous Copy's).
> > A large synchronous Copy can take a long time, resulting in a slow RPC
> > response time. A user reported 25sec.
> > The 25sec case turned out to be a ZFS specific issue that could be avoi=
ded
> > via a ZFS tunable.
> >
> > I do not have a good generic solution, but I do have a tunable that can
> > be used to clip the Copy len, which is a rather blunt and ugly workarou=
nd.
> > (The generic copy routine internal to FreeBSD can accept a flag that in=
dicates
> > "return after 1sec with a partial copy done", but that is not implement=
ed for
> > file systems like ZFS.)
> >
> > Although there is no hard limit in the RFCs for RPC response times,
> > I've typically
> > assumed a max of 1-2sec is desirable.
>
> This is not meant to be a permanent change, but rather one that can
> be backported to LTS kernels if we see the need. A long-term fix
> will re-enable async COPY but deal properly with the loss of a
> CB_OFFLOAD.
>
> The server should return NFS4ERR_DELAY if it expects to take a long
> time, no?
And then the client is going to try the same Copy again after waiting a whi=
le,
yes?
I think you need the Copy to make some progress, so that the client doesn't
just try try again. (Also, in FreeBSD it wouldn't be easy to "interrupt" a =
copy
that is in progress on a server file system so, at least in FreeBSD, I don'=
t
know how the NFS server would know to expect a long delay.  As I noted,
there is a "stop copying after 1sec" flag that, if obeyed by all file syste=
ms,
does allow some progress without taking an excessive time.)

rick

>
> > rick
> >
> > >
> > > I have some patches that add an OFFLOAD_STATUS implementation to the
> > > Linux NFS client, but that is not likely to fix older clients.
> > >
> > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > ---
> > >  fs/nfsd/nfs4proc.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > > index ea3cc3e870a7..12722c709cc6 100644
> > > --- a/fs/nfsd/nfs4proc.c
> > > +++ b/fs/nfsd/nfs4proc.c
> > > @@ -1807,6 +1807,13 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd=
4_compound_state *cstate,
> > >         __be32 status;
> > >         struct nfsd4_copy *async_copy =3D NULL;
> > >
> > > +       /*
> > > +        * Currently, async COPY is not reliable. Force all COPY
> > > +        * requests to be synchronous to avoid client application
> > > +        * hangs waiting for completion.
> > > +        */
> > > +       nfsd4_copy_set_sync(copy, true);
> > > +
> > >         copy->cp_clp =3D cstate->clp;
> > >         if (nfsd4_ssc_is_inter(copy)) {
> > >                 trace_nfsd_copy_inter(copy);
> > >
> > > base-commit: 939cb14d51a150e3c12ef7a8ce0ba04ce6131bd2
> > > --
> > > 2.44.0
> > >
> > >
>
> --
> Chuck Lever