Re: [GIT PULL] ring-buffer: Updates for v6.15

[Linus Torvalds]

 On Thu, 27 Mar 2025 at 19:44, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> When we want to map it to user space, which was added in 6.10. Before
> that struct page was hardly used.

Right. That *used* to be the situation. But that's no longer the
situation since you want to mmap things.

Again: if you remove mmap(), all this goes away.

That's an option. But instead, you added *more* mmap, and made the
problem acute by also making the code much uglier.

What used to be a small and fairly harmless hack is now ugly beyond
belief and fragile.

> I'M NOT LYING, and honestly I'm quite offended that you accused me of
> lying!

What's the definition of lying again? Saying something that wasn't
true? That's literally what you did. You claimed only the virtual
address was used. That's *CLEARLY* not true.

The fact that it *used* to be true, and that the code was originally
designed in a time when it was true is really not relevant to the
current situation, now is it?

> Before 6.10 (from 2.6.28 through to 6.9), the only uses of struct page
> was in the allocation and freeing of the ring buffer. It wasn't used
> before that. The virtual address is what the kernel writes to, it's
> what was read.

What the hell is your point?

Bringing up something that *used* to be true, but isn't any longer true?

Seriously, what's the point of that?

So face the music. The fact that you mmap the thing means that now it
needs 'struct page'. Stop making excuses, stop making things up, and
just deal with it.

or don;t.

> But when you read the "trace" or "trace_pipe" file, or even read the
> "trace_pipe_raw", the struct page is never used.

Again, WHAT IS YOUR POINT?

Those aren't the issues. The issue is that you added mmap. Stop
bringing up completely irrelevant issues. Stop saying "but but but
other code doesn't need it".

The code directly under discussion clearly does in fact need it and use it.

> Well, it wasn't that I thought it was hackery, it was that it feels like
> you are looking for anything to yell at me for at every pull request I
> do. And this one I figured you would find something with.

So next time, make sure it's clean. You clearly knew that code was questionable.

And yes, you also knew very well that I look at your pull requests
more carefully. I've made that clear before. There's a history to your
pull requests, and now we're dealing with that history.

People who don't cause issues have an easier time, because I assume
that they will *continue* to not cause issues. I've made that very
clear. It means that you need to be more careful than the average
bear.

So when you go "this will cause controversy", you should take a step
back, and think about how you can *avoid* writing things in an ugly
manner.

Instead you just went for it, and here we are.

So for next time, just don't repeat the same ugly hackery five times
in a row, for one thing. If that code had been *clean* code, I would
almost certainly not have reacted to the nasty "translate pointers
back and forth".

But as it is, instead of having some clear abstractions and helper
functions to do things in one place with sane tests for "if this is a
,TRACE_ARRAY_FL_BOOT thing, we have a clearly separate path" you
repeated the disgusting hackery overr and overr and made sure it was
front and center.

Now it's too late. I've seen that horror, and I'm not willing to take it.

You need to keep the 'struct page' around for mmap. Instead of living
in the past and saying "before mmap, we didn't need it", just realize
that the "before mmap" situation is not relevant any more. We're not
there.

Or just say "mmap just isn't worth the pain of cleaning up this code".

That's ok too.

                Linus