Re: [for-6.16-final PATCH 9/9] nfs/localio: add localio_async_probe modparm

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2025-07-14 at 14:23 +1000, NeilBrown wrote:
> On Mon, 14 Jul 2025, Mike Snitzer wrote:
> > From: Mike Snitzer <snitzer@hammerspace.com>
> > 
> > This knob influences the LOCALIO handshake so that it happens
> > synchronously upon NFS client creation.. which reduces the window of
> > opportunity for a bunch of IO to flood page cache and send out over to
> > NFSD before LOCALIO handshake negotiates that the client and server
> > are local.  The knob is:
> >   echo N > /sys/module/nfs/parameters/localio_async_probe
> 
> I understand why you are adding this but ....  yuck.  Tuning knobs are
> best avoided.
> 
> Maybe we could still do the probe async, but in mount wait for 200ms,
> or for the probe to get a reply.   That should make everyone happy.
> 
> NeilBrown
> 

Agreed. I'd prefer to not need a tuning knob for this. Doing a short
wait in the mount for localio negotiation would be a lot more
reasonable.

That said, why is the LOCALIO negotiation taking so long? Shouldn't
that be basically instantaneous during the mount? Do we really have
applications that are hammering reads and writes just after the mount
returns but before the localio negotiation completes?

> 
> > 
> > Fixes: 1ff4716f420b ("NFS: always probe for LOCALIO support asynchronously")
> > Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
> > ---
> >  fs/nfs/localio.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
> > index e3ae003118cb..76ca9bd21d2e 100644
> > --- a/fs/nfs/localio.c
> > +++ b/fs/nfs/localio.c
> > @@ -49,6 +49,11 @@ struct nfs_local_fsync_ctx {
> >  static bool localio_enabled __read_mostly = true;
> >  module_param(localio_enabled, bool, 0644);
> >  
> > +static bool localio_async_probe __read_mostly = true;
> > +module_param(localio_async_probe, bool, 0644);
> > +MODULE_PARM_DESC(localio_async_probe,
> > +		 "Probe for LOCALIO support asynchronously.");
> > +
> >  static bool localio_O_DIRECT_semantics __read_mostly = false;
> >  module_param(localio_O_DIRECT_semantics, bool, 0644);
> >  MODULE_PARM_DESC(localio_O_DIRECT_semantics,
> > @@ -203,7 +208,10 @@ void nfs_local_probe_async_work(struct work_struct *work)
> >  
> >  void nfs_local_probe_async(struct nfs_client *clp)
> >  {
> > -	queue_work(nfsiod_workqueue, &clp->cl_local_probe_work);
> > +	if (likely(localio_async_probe))
> > +		queue_work(nfsiod_workqueue, &clp->cl_local_probe_work);
> > +	else
> > +		nfs_local_probe(clp);
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_local_probe_async);
> >  
> > -- 
> > 2.44.0
> > 
> > 

-- 
Jeff Layton <jlayton@kernel.org>