Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
From:   Benjamin Coddington <bcodding@redhat.com>
To:     Anna Schumaker <anna@kernel.org>
Cc:     Chuck Lever <chuck.lever@oracle.com>,
        Jeff Layton <jlayton@kernel.org>,
        trond.myklebust@hammerspace.com, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 2/2] NFSv4: Allow per-mount tuning of READDIR attrs
Date:   Wed, 18 Oct 2023 15:08:45 -0400
Message-ID: <828F5AC6-99CA-417A-9475-41B6B9F35DB1@redhat.com>
In-Reply-To: <CAFX2JfkON+5MsYuw-SsvSg04M6Fy=BY_v7RZ9aAs35P7fD15Gw@mail.gmail.com>
References: <cover.1697577945.git.bcodding@redhat.com>
 <bd900de1d19bc56e6df5b44379f373617acc894e.1697577945.git.bcodding@redhat.com>
 <ZS/V+4Cuzox7erqz@tissot.1015granger.net>
 <6157b73e380e5b625cd8ed0133ef392d0dd4bd8b.camel@kernel.org>
 <ZS/qwYzQvrgJNEv6@tissot.1015granger.net>
 <CAFX2JfkON+5MsYuw-SsvSg04M6Fy=BY_v7RZ9aAs35P7fD15Gw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 18 Oct 2023, at 14:38, Anna Schumaker wrote:

> On Wed, Oct 18, 2023 at 10:25 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>>
>> On Wed, Oct 18, 2023 at 09:33:40AM -0400, Jeff Layton wrote:
>>> On Wed, 2023-10-18 at 08:56 -0400, Chuck Lever wrote:
>>>> On Tue, Oct 17, 2023 at 05:30:44PM -0400, Benjamin Coddington wrote:
>>>>> Expose a per-mount knob in sysfs to set the READDIR requested attributes
>>>>> for a non-plus READDIR request.
>>>>>
>>>>> For example:
>>>>>
>>>>>   echo 0x800 0x800000 0x0 > /sys/fs/nfs/0\:57/v4_readdir_attrs
>>>>>
>
> I understand why you're not adding a keyword for each attribute
> requested in a readdir, but would it be possible to have a single
> magic keyword like "reset" or "default" that is an alias for reverting
> to the default attributes?

Yes, it's possible.  But what happens when we change the defaults again?
"Reset" becomes meaningless after that.  That sort of sysfs addition is not
future-proof.  This file both shows the current and any future default set
of attributes being used on the client as well as allowing them to be
modified.

The only attributes that are allowed to be set are those that the client
would already request and properly decode in the readdir plus path.  The
foot-shooty space is the permutation of every combination of those 20
attributes, save the three cases we've been stomping on already: 1) the
non-plus case, 2) the new non-plus with type, and 3) the plus case with all
20 attributes.

I suppose I could test all those cases for weirdness, but I expect they'd
all "just work" for listing directories. (I have tested quite a few without
surprises.)  Perhaps some cases would expose assumptions in the attribute
cache on the client -- for example the client might expect _SIZE and
_SPACE_USED to always be updated in the same operation.  But, I don't expect
that to create devastating issues, and I really don't think anyone's going
to need to break the client by trying to ask for only _SIZE without
_SPACE_USED (all hypothetical).

Another way forward may be to just allow the addition or removal of _TYPE,
but the client I want to use allows me to request any of those attributes.
If this never ends up helping anyone, I'll eat my hat.

Ben