Linux NFS development
 help / color / mirror / Atom feed
From: "Anna Schumaker" <anna@kernel.org>
To: "Mike Snitzer" <snitzer@kernel.org>
Cc: "Trond Myklebust" <trondmy@kernel.org>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 3/3] NFS/localio: give the LOCALIO probe a short timeout of its own
Date: Mon, 09 Nov 2026 10:57:39 -0500	[thread overview]
Message-ID: <66beadbd-5735-4941-821e-059d5cb9850a@app.fastmail.com> (raw)
In-Reply-To: <avHiX4PgUIEQFbX2@kernel.org>

Hi Mike,

On Mon, Nov 9, 2026, at 10:10 AM, Mike Snitzer wrote:
> On Fri, Nov 06, 2026 at 05:02:09PM -0500, Anna Schumaker wrote:
>> 
>> 
>> On Mon, Nov 2, 2026, at 4:52 PM, Mike Snitzer wrote:
>> > The LOCALIO probe clones the NFS client with rpc_bind_new_program(),
>> > which NULL-pings the new program before the probe sends UUID_IS_LOCAL.
>> > Both calls use the mount's timeout.  With timeo=600 and the default
>> > retrans, a server that does not answer the LOCALIO program holds each
>> > probe for about 170 s.  The client then logs "nfslocalio: server ... not
>> > responding, timed out".  On such a server, one mkdir took about 170 s
>> > of a 202 s run and ended when the probe gave up.
>> >
>> > Give the probe a soft timeout of its own, with no retries and no log
>> > message.  The new nfs module parameter localio_timeout sets it in
>> > seconds, 10 by default and at most 600.  Set it on a private clone of
>> > the NFS client before rpc_bind_new_program(), so that the ping uses it
>> > as well.  Clear cl_noretranstimeo, which NFSv4 mounts set, because with
>> > it a sent call does not time out while the connection stays up.  With
>> > localio_timeout set to 0, the probe uses the NFS client's timeouts, as
>> > before.  The probe still shares the NFS client's transport and runs
>> > beside its RPCs.  The NFS client's own timeouts do not change.
>> >
>> > Tested through a proxy that drops every call to the LOCALIO program:
>> > the probe gave up after 10.6 s and logged nothing.  With
>> > localio_timeout=0 it took 182.9 s and logged the message above.  The
>> > probe still succeeds against a server that supports LOCALIO: with no
>> > proxy, a 64 MiB O_DIRECT write through LOCALIO sent no WRITE RPCs.
>> >
>> > Signed-off-by: Mike Snitzer <snitzer@kernel.org>
>> > ---
>> >  Documentation/filesystems/nfs/localio.rst |  9 ++++
>> >  fs/nfs/localio.c                          | 54 +++++++++++++++++++++--
>> >  2 files changed, 60 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/Documentation/filesystems/nfs/localio.rst 
>> > b/Documentation/filesystems/nfs/localio.rst
>> > index e8f44332cc806..45c6981da0d89 100644
>> > --- a/Documentation/filesystems/nfs/localio.rst
>> > +++ b/Documentation/filesystems/nfs/localio.rst
>> > @@ -316,6 +316,15 @@ It is checked on every I/O: setting it to N sends 
>> > the next I/O of every
>> >  client over the network, and setting it back to Y resumes LOCALIO.  A
>> >  client that probed while it was N is probed again when it is set to Y.
>> > 
>> > +/sys/module/nfs/parameters/localio_timeout (uint)
>> > +sets how many seconds the LOCALIO probe waits for each reply from the
>> > +server, defaults to 10, at most 600.  A server that does not support
>> > +LOCALIO may not answer the probe at all.  If no reply comes in time,
>> > +the probe gives up without a log message.  The client then does not use
>> > +LOCALIO.  The mount's timeouts do not apply to the probe.  Set to 0, the
>> > +probe uses the mount's timeouts instead, as an NFS RPC would.  A change
>> > +applies to the next probe.
>> > +
>> >  /sys/module/nfs/parameters/localio_O_DIRECT_semantics (bool)
>> >  controls if O_DIRECT extends down to the underlying filesystem, defaults
>> >  to N. Application IO must be logical blocksize aligned, otherwise
>> > diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
>> > index 80a496d6fe0d6..06d0755b1f203 100644
>> > --- a/fs/nfs/localio.c
>> > +++ b/fs/nfs/localio.c
>> > @@ -82,6 +82,22 @@ static const struct kernel_param_ops localio_enabled_ops = {
>> >  };
>> >  module_param_cb(localio_enabled, &localio_enabled_ops, &localio_enabled, 0644);
>> > 
>> > +/* Seconds the LOCALIO probe waits for each reply; 0: the NFS client's 
>> > own */
>> 
>> I'm having trouble parsing this. What does '0' mean here?
>> 
>> Thanks,
>> Anna
>
> Sorry about that, not sure what happened.  But how 0 is handled is
> documented in localio.rst (above) and in the comment above
> nfs_init_localioclient() (below).  In any case, this comment can be:
>
>   /*
>    * Seconds the LOCALIO probe waits for each reply.  With 0, the probe uses
>    * the NFS client's timeouts instead.
>    */
>
> Can you fold this in or should I send v2?

That reads much better to me. I'll fold it into the patch!

Anna

>
> Thanks,
> Mike
>
>
>> > @@ -156,13 +172,45 @@ const struct rpc_program nfslocalio_program = {
>> > 
>> >  /*
>> >   * nfs_init_localioclient - Initialise an NFS localio client connection
>> > + *
>> > + * A server that does not support LOCALIO may not answer its RPCs at all.
>> > + * The probe shares the NFS client's transport and runs beside its RPCs.
>> > + * With localio_timeout set, it gets a quiet, soft timeout of that many
>> > + * seconds of its own, whatever timeouts the mount uses.  With 0, it uses
>> > + * the NFS client's timeouts, as an NFS RPC would.
>> >   */
>> >  static struct rpc_clnt *nfs_init_localioclient(struct nfs_client *clp)
>> >  {
>> > -	struct rpc_clnt *rpcclient_localio;
>> > +	unsigned int timeout_secs = READ_ONCE(localio_timeout);
>> > +	struct rpc_clnt *parent = clp->cl_rpcclient;
>> > +	struct rpc_clnt *probe = NULL, *rpcclient_localio;
>> > 
>> > -	rpcclient_localio = rpc_bind_new_program(clp->cl_rpcclient,
>> > -						 &nfslocalio_program, 1);
>> > +	/*
>> > +	 * rpc_bind_new_program() pings the new program with the timeout of
>> > +	 * the client it clones, so set the probe's timeout on a private
>> > +	 * clone first.  The clone's cl_timeout points at the transport's
>> > +	 * timeout: give it its own copy rather than write through it.
>> > +	 */
>> > +	if (timeout_secs) {
>> > +		probe = rpc_clone_client(parent);
>> > +		if (IS_ERR(probe))
>> > +			return probe;
>> > +		probe->cl_timeout_default = (struct rpc_timeout) {
>> > +			.to_initval	= timeout_secs * HZ,
>> > +			.to_maxval	= timeout_secs * HZ,
>> > +			.to_retries	= 0,
>> > +		};
>> > +		probe->cl_timeout = &probe->cl_timeout_default;
>> > +		probe->cl_softrtry = 1;
>> > +		probe->cl_noretranstimeo = 0;
>> > +		probe->cl_chatty = 0;
>> > +		parent = probe;
>> > +	}
>> > +
>> > +	rpcclient_localio = rpc_bind_new_program(parent, &nfslocalio_program, 1);
>> > +	/* The new client holds a reference to its parent, the probe clone */
>> > +	if (probe)
>> > +		rpc_shutdown_client(probe);
>> > 
>> >  	dprintk_rcu("%s: server (%s) %s NFS LOCALIO.\n",
>> >  		__func__, rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_ADDR),
>> > -- 
>> > 2.52.0

      reply	other threads:[~2026-11-09 15:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-02 21:52 [PATCH 0/3] NFS/localio: make the LOCALIO probe safe, repeatable and bounded Mike Snitzer
2026-11-02 21:52 ` [PATCH 1/3] NFS/localio: hold the client across an asynchronous LOCALIO probe Mike Snitzer
2026-11-02 21:52 ` [PATCH 2/3] NFS/localio: probe again for LOCALIO when localio_enabled is set to Y Mike Snitzer
2026-11-02 21:52 ` [PATCH 3/3] NFS/localio: give the LOCALIO probe a short timeout of its own Mike Snitzer
2026-11-06 22:02   ` Anna Schumaker
2026-11-09 15:10     ` Mike Snitzer
2026-11-09 15:57       ` Anna Schumaker [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=66beadbd-5735-4941-821e-059d5cb9850a@app.fastmail.com \
    --to=anna@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=snitzer@kernel.org \
    --cc=trondmy@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox