2023-09-03 13:39:46

by Chuck Lever III

[permalink] [raw]
Subject: Re: [RFC PATCH] NFSv4: add sysctl for setting READDIR attrs



> On Aug 31, 2023, at 1:03 PM, Benjamin Coddington <[email protected]> wrote:
>
> On 31 Aug 2023, at 12:28, Jeff Layton wrote:
>> This doesn't seem worthwhile to me. We have a clear reason to add
>> WORD0_TYPE to "basic" READDIR, which is that we want to be able to fill
>> out the d_type for getdents.
>
> Yeah, and exposing all the bits might create some interesting effects.

I'm wary of adding a permanent knob, but I don't see a problem
leaving this patch out on the mailing list if there are one or
two folks who will actively try this out in a multi-server-
implementation environment (say, at bake-a-thon).

There are two sides to the trade-off:

- If the client can pick up more information in a READDIR,
there might be other places where it doesn't need another
synchronous round trip. (and of course, watch out for any
corner cases that might trigger a CB_RECALL/DELEGRETURN).

- A particular server implementation (I recall OnTAP had an
issue) might not have a large cache for file attributes,
and that would turn requests for additional attributes into
extra multiple short metadata reads in the server's filesystem.

Performance studies would be interesting, and I bet this is
something a grad student could experiment with using one of
the "captured trace" workloads that academics have floating
around (or at least they used to have them).


>> I don't see the same sort of rationale for fetching other attributes. It
>> would just be priming the inode cache with certain info. That could
>> useful for some workloads, but that seems like sort of a niche thing.
>
> The issues I frequently see around READDIR are that we keep micro-optimizing
> and regressing in another place. If we set WORD0_TYPE, there's a non-zero
> chance someone might start yelling about it in awhile because their server
> takes longer to query the inode. Its nice we have the option to give the
> power back the user sometimes without needing to grow a mount option, or use
> a module param (which would appply to the whole system) - so this was a fun
> example.
>
>> Adding more info also reduces the number of entries you can pack into a
>> READDIR reply, which is makes it easier to trigger cookie problems with
>> creates and deletes in large directories.
>
> I don't think those two things are related for filesystems with stable
> cookies, or I'm not understanding you.
>
> I'm in favor of the default READDIR asking for type.

I say test/measure first, but I don't object to the idea.


--
Chuck Lever