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

[Steven Rostedt]

On Thu, 27 Mar 2025 20:05:14 -0700
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> 
> > 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.

This is more of a confusing of the words. I did say "that's all the
code cares about", I meant the code that is used for recording and
reporting the events. I thought it was obvious that I didn't mean the
mmapped code. It wasn't lying, it was a miscommunication.

> 
> > 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?

My point was that I can't *just* use the struct page. It sounds to me
that you want me to to refactor all of the code to handle struct page
even when the consumer is in the kernel.

> 
> 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.

Again, are you suggesting that the code needs to be converted to using
struct page for all of it? Even for the kernel consumer code?

> 
> 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.

I guess, are you asking to save the struct page for every allocation,
and not using virt_to_page() to get it? In other words, do I need to
increase the memory footprint to add another pointer because the code
can be mmapped?

	page = page_alloc();
	bpage->page = page_address(page);
	bpage->struct_page = page;

??

> 
> 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.

Because this was the code we last had an argument about, which is why I
figured it would be "controversial". You don't need to read more into
it than that.

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

Again, it wasn't because I thought it was hackery, but because this is
the code we last argued about. Hence, why I thought it was the most
"controversial".

> 
> 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".

So you are saying that you want this to save the page struct directly
then. My only concern with that is the added memory to save the
structure when it could be easily calculated.

As I work on memory limited devices, I try not to waste memory when
there's another way to get what is needed.

-- Steve