Received-SPF: pass (google.com: domain of linux-nfs+bounces-2818-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 15 Apr 2024 20:00:55 +0200
From: Petr Vorel <pvorel@suse.cz>
To: Jeff Layton <jlayton@kernel.org>
Cc: Chuck Lever III <chuck.lever@oracle.com>,
	"ltp@lists.linux.it" <ltp@lists.linux.it>,
	Neil Brown <neilb@suse.de>, Cyril Hrubis <chrubis@suse.cz>,
	Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 1/1] proc01: Whitelist /proc/fs/nfsd/nfsv4recoverydir
Message-ID: <20240415180055.GA557009@pevik>
Reply-To: Petr Vorel <pvorel@suse.cz>
References: <20240415172133.553441-1-pvorel@suse.cz>
 <7A48C70E-BAAB-4A1C-A41B-ABC30287D8B7@oracle.com>
 <d895ad115632912df228913d4a86e7f597b22599.camel@kernel.org>
 <6820832A-9F38-4DE7-8EE4-7AAC8CF06FD4@oracle.com>
 <5052616ca4c2789ffcc51a27cbff060e2fbdb7b4.camel@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <5052616ca4c2789ffcc51a27cbff060e2fbdb7b4.camel@kernel.org>

> On Mon, 2024-04-15 at 17:37 +0000, Chuck Lever III wrote:

> > > On Apr 15, 2024, at 1:35 PM, Jeff Layton <jlayton@kernel.org> wrote:

> > > On Mon, 2024-04-15 at 17:27 +0000, Chuck Lever III wrote:

> > > > > On Apr 15, 2024, at 1:21 PM, Petr Vorel <pvorel@suse.cz> wrote:

> > > > > /proc/fs/nfsd/nfsv4recoverydir started from kernel 6.8 report EINVAL.

> > > > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > > > ---
> > > > > Hi,

> > > > > @ Jeff, Chuck, Neil, NFS devs: The patch itself whitelist reading
> > > > > /proc/fs/nfsd/nfsv4recoverydir in LTP test. I suspect reading failed
> > > > > with EINVAL in 6.8 was a deliberate change and expected behavior when
> > > > > CONFIG_NFSD_LEGACY_CLIENT_TRACKING is not set:

> > > > I'm not sure it was deliberate. This seems like a behavior
> > > > regression. Jeff?


> > > I don't think I intended to make it return -EINVAL. I guess that's what
> > > happens when there is no entry for it in the write_op array.

> > > With CONFIG_NFSD_LEGACY_CLIENT_TRACKING disabled, that file has no
> > > meaning or value at all anymore. Maybe we should just remove the dentry
> > > altogether when CONFIG_NFSD_LEGACY_CLIENT_TRACKING is disabled?

> > My understanding of the rules about modifying this part of
> > the kernel-user interface is that the file has to stay, even
> > though it's now a no-op.

First, thanks a lot for handling this.

> Does it? Where are these rules written? 

I wonder myself as well.

> What should we have it do now when read and written? Maybe EOPNOTSUPP
> would be better, if we can make it just return an error?

FYI current exceptions on /proc files in whole kernel have various errnos, e.g.
EINVAL, EOPNOTSUPP:
https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/fs/proc/proc01.c#L81

Kind regards,
Petr

> We could also make it just discard written data, and present a blank
> string when read. What do the rules say we are required to do here?