All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.