On Thu, Sep 04, 2025 at 06:50:29PM -0700, Darrick J. Wong wrote:
> On Thu, Sep 04, 2025 at 05:17:13PM +0200, Miklos Szeredi wrote:
> > On Thu, 4 Sept 2025 at 16:45, Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
> >
> > > Or do you prefer the first N patches to include only code and no
> > > debugging stuff at all, with a megapatch at the end to add all that
> > > instrumentation?
> >
> > It doesn't have to be a megapatch, could be a nicely broken up series.
> > But separate from the main patchset, if possible.
>
> I'll give it a try.
Hi Miklos!
I gave it a try for V5 and the results were ... not fun.
I. Moving all the tracepoint definitions to come before the beginning of
code changes breaks compilation in three critical ways:
(a) The tracepoints take as arguments pointers to structs that only get
defined later in the patchset.
(b) Tracepoints that extract data from those structs cannot do so
because (obviously) they are not actually defined yet.
(b) The flag constants to strings decoding mechanism requires those
constants to be defined.
II. I fixed /that/ by moving the tracepoint definitions to come after
the all code change, but that brings its own problems:
(1) Now I have to move all the callsites from the code patches to this
new patch at the end.
(2) I've found that oftentimes, if I need to change one of the code
patches to fix a bug, there will be a merge conflict in the
tracepoint patch at the end because some whitespace moved, etc.
That's annoying to have to fix up every time I make a change, the
suggested conflict resolution puts the trace_() call in completely
the wrong place because well, textually it looked right.
(3) If I change a structure in a code patch, I'm not going to find out
if it breaks the tracepoint until I get to that megapatch. In the
good case I can just fix the tracepoint, but in the bad case I have
to go all the way back to the code patch to fix that, then rebase
everything back to the tracepoint patch. Right now, that kind of
breakage is immediately obvious when building each code patch.
III. A middle ground that I devised is to have a patch N that adds code,
and patch N+1 adds the tracepoints. That avoids most of the problems of
the first two approaches, but now there are twice as many patches, e.g.:
* fuse: implement direct IO with iomap
* fuse: add tracepoints for implement direct IO with iomap
Coming from XFS, I encountered a lot of friction from some reviewers for
putting too many things in a single patch, and from other reviewers for
sending too many patches once I split things up. I'd like to avoid that
here.
So I think the third approach is the way to go, but I'd like your input
before I commit to a particular approach. Just to be clear, I'm trying
to avoid landing any of this in 6.18 (aka presumed 2025 LTS), so there's
plenty of time.
--D
