On Mon, 14 Jul 2025, Mike Snitzer wrote:
> From: Mike Snitzer <snitzer@xxxxxxxxxxxxxxx>
>
> Previously nfs_local_probe() was made to disable and then attempt to
> re-enable LOCALIO (via LOCALIO protocol handshake) if/when it was
> called and LOCALIO already enabled.
>
> Vague memory for _why_ this was the case is that this was useful
> if/when a local NFS server were to be restarted with a local NFS
> client connected to it.
>
> But as it happens this causes an absurd amount of LOCALIO flapping
> which has a side-effect of too much IO being needlessly sent to NFSD
> (using RPC over the loopback network interface). This is the
> definition of "serious performance loss" (that negates the point of
> having LOCALIO).
>
> So remove this mis-optimization for re-enabling LOCALIO if/when an NFS
> server is restarted (which is an extremely rare thing to do). Will
> revisit testing that scenario again but in the meantime this patch
> restores the full benefit of LOCALIO.
>
> Fixes: 56bcd0f07fdb ("nfs: implement client support for NFS_LOCALIO_PROGRAM")
> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxxxxxxx>
Reviewed-by: NeilBrown <neil@xxxxxxxxxx>
I cannot see any justification for probing if localio is currently
thought to be working. If for some reason it doesn't work, then when we
notice that is a good time to disable - which it what this patch does.
However...
> ---
> fs/nfs/localio.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
> index a4bacd9a5052..e3ae003118cb 100644
> --- a/fs/nfs/localio.c
> +++ b/fs/nfs/localio.c
> @@ -180,10 +180,8 @@ static void nfs_local_probe(struct nfs_client *clp)
> return;
> }
>
> - if (nfs_client_is_local(clp)) {
> - /* If already enabled, disable and re-enable */
> - nfs_localio_disable_client(clp);
> - }
> + if (nfs_client_is_local(clp))
> + return;
>
> if (!nfs_uuid_begin(&clp->cl_uuid))
> return;
> @@ -241,7 +239,8 @@ __nfs_local_open_fh(struct nfs_client *clp, const struct cred *cred,
> case -ENOMEM:
> case -ENXIO:
> case -ENOENT:
> - /* Revalidate localio, will disable if unsupported */
> + /* Revalidate localio */
> + nfs_localio_disable_client(clp);
> nfs_local_probe(clp);
Shouldn't that be nfs_local_probe_async() ???? I wonder why that wasn't
changed in
Commit 1ff4716f420b ("NFS: always probe for LOCALIO support asynchronously")
Thanks,
NeilBrown
