Received-SPF: pass (google.com: domain of linux-nfs+bounces-2864-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 17 Apr 2024 07:25:16 +0200
From: Petr Vorel <pvorel@suse.cz>
To: NeilBrown <neilb@suse.de>
Cc: Chuck Lever <chuck.lever@oracle.com>, Jeff Layton <jlayton@kernel.org>,
	Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	linux-nfs@vger.kernel.org
Subject: Re: [PATCH] nfsd: don't create nfsv4recoverydir in nfsdfs when not
 used.
Message-ID: <20240417052516.GA681570@pevik>
Reply-To: Petr Vorel <pvorel@suse.cz>
References: <171330258224.17212.9790424282163530018@noble.neil.brown.name>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <171330258224.17212.9790424282163530018@noble.neil.brown.name>

Hi Neil, all,

> When CONFIG_NFSD_LEGACY_CLIENT_TRACKING is not set, the virtual file
>   /proc/fs/nfsd/nfsv4recoverydir
> is created but responds EINVAL to any access.
> This is not useful, is somewhat surprising, and it causes ltp to
> complain.

> The only known user of this file is in nfs-utils, which handles
> non-existence and read-failure equally well.  So there is nothing to
> gain from leaving the file present but inaccessible.

> So this patch removes the file when its content is not available - i.e.
> when that config option is not selected.

> Also remove the #ifdef which hides some of the enum values when
> CONFIG_NFSD_V$ not selection.  simple_fill_super() quietly ignores array
> entries that are not present, so having slots in the array that don't
> get used is perfectly acceptable.  So there is no value in this #ifdef.

> Reported-by: Petr Vorel <pvorel@suse.cz>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Fixes: 74fd48739d04 ("nfsd: new Kconfig option for legacy client tracking")
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/nfsd/nfsctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 93c87587e646..340c5d61f199 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -48,12 +48,10 @@ enum {
>  	NFSD_MaxBlkSize,
>  	NFSD_MaxConnections,
>  	NFSD_Filecache,
> -#ifdef CONFIG_NFSD_V4
>  	NFSD_Leasetime,
>  	NFSD_Gracetime,
>  	NFSD_RecoveryDir,
>  	NFSD_V4EndGrace,
> -#endif
>  	NFSD_MaxReserved
>  };

> @@ -1360,7 +1358,9 @@ static int nfsd_fill_super(struct super_block *sb, struct fs_context *fc)
>  #ifdef CONFIG_NFSD_V4
>  		[NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops, S_IWUSR|S_IRUSR},
>  		[NFSD_Gracetime] = {"nfsv4gracetime", &transaction_ops, S_IWUSR|S_IRUSR},
> +#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
>  		[NFSD_RecoveryDir] = {"nfsv4recoverydir", &transaction_ops, S_IWUSR|S_IRUSR},
> +#endif

LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

>  		[NFSD_V4EndGrace] = {"v4_end_grace", &transaction_ops, S_IWUSR|S_IRUGO},
>  #endif
>  		/* last one */ {""}