Received-SPF: pass (google.com: domain of linux-nfs+bounces-2882-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 17 Apr 2024 12:28:21 -0400
From: Chuck Lever <chuck.lever@oracle.com>
To: NeilBrown <neilb@suse.de>
Cc: Petr Vorel <pvorel@suse.cz>, 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: <Zh/4pd4stFgjV7gp@tissot.1015granger.net>
References: <171330258224.17212.9790424282163530018@noble.neil.brown.name>
 <20240417052516.GA681570@pevik>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240417052516.GA681570@pevik>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?wv/+pMNIgFJsyMLGutDP2XR7PMQNVJWKNfEeBypRWG7x7NX0D+0ZE+t+ZD5r?=
 =?us-ascii?Q?wGpy6TLUXSk+tQDcdGS8IBqQA3Fzk0uUz3Upx9sgHao7ap/gZ7YOQoNvfrkv?=
 =?us-ascii?Q?82ub6GDLcDYRUkcooJg4COXlrzlCAGcwWWGLfV123pOPJQehMrguHEXJ9/mq?=
 =?us-ascii?Q?0iR2zMeF3F8Rvn2m5jRQW3AUHvVemx/0Pp2If7VD4s97WMTQ/kj/Pik6iEo8?=
 =?us-ascii?Q?jnoCa1fRT+WNfUEnNrfAs8Orzvyx0YchlPtoSSeCZZ785PjLjnDxeO4G8pqk?=
 =?us-ascii?Q?n7KHXEbneRGlwiVJR8poNgL9OBa9Ut1OKhtvIbc+PEOYTaaXxP/kB+RzQh+w?=
 =?us-ascii?Q?nxkY37fabldGLPSX9oZb+0JYyv07/MaBym7DBQMAQCJQgS1lewdN5OypoTKu?=
 =?us-ascii?Q?cab33HoEmNN2RnNjXXBBh+B0feJiL+NjvRaDoXdMMzExUlyQrE8QoSuPOGgB?=
 =?us-ascii?Q?zDbiGEoBA2/eMC/QCpAoJK7W4zY9kKgmiDcZIKLTVcRh69dlHIKVIeOm2pk/?=
 =?us-ascii?Q?UGo7/wNdhBD1O/AFJmDmY7AeYy8fWDoWYJvP0fXOIHg++wmE+GAPd3EK78gO?=
 =?us-ascii?Q?SqeCgTUjREG9xO3HzbOJr1zbESK7vDOpjJKL1zcjuiG4uBhSrThaU7gGpKQ0?=
 =?us-ascii?Q?Yqtqy8EuKSr+g9ULSS3OAKEwpQKB3MZzLqVmxbvwEayyrCXG4CvRjyn8R4Vw?=
 =?us-ascii?Q?ikHlfElBXOd+yK0aZ0UtUdZa8HOjAZhO+UGMiaAgoTDXoa2Nl5byQZfiP0jV?=
 =?us-ascii?Q?bNCTsO+EQxGarJlQsHw2xwB8mR8OgJd9PlnaG2bScoqkY2ykZK5DO855ILaB?=
 =?us-ascii?Q?3cvJ79wbdcPqHBfo3gTi7t442xguTsBLT0pngPPaK2S2tS/Sv0e24NbZOzP9?=
 =?us-ascii?Q?ESoZJpn8L/loFshRELQznA+nLVp0d3zg6HLmv5w6CDPFAdTRIcQymXvfZWG4?=
 =?us-ascii?Q?PcgMPbJDUy9uQB4krH6fOVq4VciyyRUsi24UxnkmrtBL3rM+0bm2/64tQ1PG?=
 =?us-ascii?Q?cbBIODUJljFQEAZgobGOYP98vssVN9hAgDx2J6fZnd8qGoZxoJUByT37bL43?=
 =?us-ascii?Q?ediJrkTp4lp8ksRdXUiQLwv8W5E3+miqTN+1F4/+CbSeuiUXxyw387U0hruy?=
 =?us-ascii?Q?vn++BPpQsZdOjZrWu4/oKB2IpbIzwxoHycONSvc2jwNr9XzKJF1ZQqxqBH31?=
 =?us-ascii?Q?+lemWx/DklnVpPdSTQRtjX/rmpuoCsa8cO7DZoEnU9S79fxya48qOLQV3dke?=
 =?us-ascii?Q?sTpaFTSY/w1dC2JD5gS9ypVOaYnPveTTgswkRtGy9QSCsvJgP/70eU65hIth?=
 =?us-ascii?Q?+exs0SEVZQDg+oHDAHnNnQwBjotlkGhD0CDm8QyVjk9+7SGujYiZOpt457tP?=
 =?us-ascii?Q?kyGsZerwVqJ27KN3hiRsggDXM9XP5kUW5GUn8kqmohhK0L9MjGBjJPAIVnjQ?=
 =?us-ascii?Q?ufqrvQm+hyzANo/r5JQJZ/dODRTbbFyEijB5D0eporutYfiIHdvcnC/DVv+r?=
 =?us-ascii?Q?kRyVA48HvMWs3TsSYpigGHvJeQP6vCo80usq/FFo1e4WUeu98WQYE0gKWU18?=
 =?us-ascii?Q?Y4Q76y5BNAM9xPjBJ0O3ZsuclLRU4XjoGfgqME/HDFQqUaZXeM34Xe3KpSak?=
 =?us-ascii?Q?Qg=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	a9wInc1tR+l/YP+hl6RGhoQI7CsWfibO0GNCoIJqAovCZGTZ+hDz64PYI/+6qiC7TiVDK+1NAMb8Nx6O0SKUYGJtQma05aODcYlKBsxNXJnJ+J7ENbJ4K9X+jzlbyB6EUCTmd3qXtcqa+mhY7ycHVurZ8GQwgpBmIWpYBBQTEQk5sa05a3GzsZ0WZ+iMUD+QNnjSe7FvCuA9ZqYyoqqS5URY9BlGn2Qna0WEDHGvjUgcaMotN2nS5Z5C2ZsP0FmSyDMI52E+ltniMy4Nj/l+hEyDNSePc025dSJMGHVTCvZwwB0lcbqt5a0hm3NHJJwKDbp11PgxpaAP/OYxbcLNyZI7ov6UwthqU4k/2uJ01md7qxStVWZ/h89KQdC8II2dRKs9cNvV1gX3qayYFtpxW65skmnQA5wIi2jKGJ485kG9rqC1Y0YwPwN5ujBw3R7DeyeA0CeeOHhpupiTefxd0pOErTV0JHg81OxXDqwxvTLSzhRodND5xs26FhZu2rBDF/0ONN07jJOnN9MZY4aVT3mkKFi/v5SKKAIziC8RQ3B6AQlcd7BJGfIVto8/M1jctW8Ifxhnr5U7SzOQTwwVG3FNHoa8sHqL6mFEYWQ0aWs=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 486e7628-27bb-4e6b-ffdd-08dc5efb6704
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Apr 2024 16:28:23.9815
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: oHe/siZx7fMaQXK3fLiQS/e+UkIbUAVhi/cqrk2wRxt51UQquM+xCTBXCmxfDFVavp+5a2cB6EoKQej0zl1r1Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB7411
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-04-17_14,2024-04-16_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxscore=0 mlxlogscore=999
 suspectscore=0 adultscore=0 phishscore=0 bulkscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000
 definitions=main-2404170115
X-Proofpoint-ORIG-GUID: XvzkjYcWVV77e4dgl9Dcwfj_7YaY_4Ls
X-Proofpoint-GUID: XvzkjYcWVV77e4dgl9Dcwfj_7YaY_4Ls

On Wed, Apr 17, 2024 at 07:25:16AM +0200, Petr Vorel wrote:
> 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 */ {""}

Thanks to you both! Applied to nfsd-next (for v6.10), unless you'd
like this to go in sooner.

-- 
Chuck Lever