> On Aug 31, 2023, at 1:03 PM, Benjamin Coddington <bcodding@xxxxxxxxxx> 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
