Re: [PATCH] io: adjust io provider for NFS tracepoint variants

[Nick Alcock <nick.alcock@oracle.com>]

On 8 Jan 2025, Eugene Loh uttered the following:

> You already have a R-b, it seems the code is right (woo hoo! cool), and we're trying to get a release out the door.  But here are my
> "late to the party" comments.  Feel free to ignore.

Ditto :)

> The names v1 and v2 strike me as funny;  they just make up new numbers.  Not a big deal, but ideally the names might more closely
> reflect the actual version changes we're tracking.

Agreed.

> Breaking the function out into a new "v1" version is some unneeded copy-and-paste code bloat, I think.  The function is kind of
> large and the only difference between v1 and v2 is I guess in the two, relatively short, "start" sections.  So, keeping this stuff
> as one function -- with extra logic in the two "start" sections -- might be more compact and clearer than having two rather similar
> functions.

Agreed, but more generally it seems we are growing increasingly many
fairly similar fake bio functions in the io provider: with this I think
we're up to three. Maybe it's time to refactor some of this into some
sort of shared fake bio machinery? Certainly if we grow more I think we
should (and given the number of network filesystems, that seems quite
likely to happen).

> On 1/7/25 17:44, Kris Van Hees wrote:
>> Kernels prior to 5.6.0 pass 3 arguments (derived from the NFS hdr)
>> to the nfs_initiate_read raw tracepoint, whereas kernels as of 5.6.0
>> pass just the NFS hdr.
>
> Is the same true of write?  If so, then maybe say so and point out that what we're really doing is changing the handling of nfs
> "start" while leaving nfs "done" alone...  that would correspond more closely to what is happening in the code, which talks of start
> and done.

Good thinkiing. That would be commit
5bb2a7cb9fe58d2b1efedd6058d442c7871c45ec ("NFS: Clean up generic
writeback tracepoints"), also by Trond, also first landing in 5.6-rc1.
Same sort of thing.