Re: [PATCH 1/1] proc01: Whitelist /proc/fs/nfsd/nfsv4recoverydir

[Petr Vorel <pvorel@suse.cz>]

> 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?