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

[Steven Rostedt]

On Thu, 27 Mar 2025 19:17:34 -0700
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Thu, 27 Mar 2025 at 19:01, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> >
> > Let me explain this better. Yes it uses alloc_page() but that's just an
> > intermediate function. What is saved is the page_address(page), as that
> > is what is used by the code.  
> 
> I understood.
> 
> But NO, that is *not* what is used by the code.
> 
> You literally use "struct page *" whenever you want to mmap it.

When we want to map it to user space, which was added in 6.10. Before
that struct page was hardly used.

> 
> If the *only* thing that was used was the virtual address, this
> wouldn't be a discussion.
> 
> As it is, you allocate it in form A, then you turn it into *either*
> form B or C, and then when you need form A again you turn it back. And
> you do so in a particularly disgusting and random way.
> 
> > The virtual address needs to be created as that's all the code cares
> > about.  
> 
> Stop LYING.

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

I make it a point to point out very clearly what I'm doing and have
never tried to sneak something past you. That's why our arguments have
been from the cover letter and not the patches themselves.

> 
> If that was "all the code cared about" we'd not have this discussion.
> 
> It clearly wants things *back* as 'struct page' again. Stop making
> shit up and maki9ng excuses for your bad code.

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.

We just started allowing the ring buffer to be memory mapped by user
space in 6.10. But that's not even the main way the buffer is read.
Most of the time, the buffer is read by the "trace" file and that is
where the kernel translates the buffer into human readable text. That
most definitely doesn't need the struct page.

Note, in the splice case, the tracing code does convert it back to a
struct page. But if you do splice on a memory mapped buffer, the ring
buffer code allocates a new page and copies it, as the memory mapped
buffer can not be passed to splice as that requires giving up ownership.

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

> 
> And yes, it would probably be fine to just keep it as *both* in parallel.
> 
> But honestly, if you only save one thing - as you currently do - you
> should save the thing that is more fundamental.
> 
> That's 'struct page *'. That's the allocation, that's the thing you
> need for mmap, and that's what you *should* have used for
> de-allocation too (even if I think you currently just end up using the
> virtual mapping and depend on the VM code undoing your mapping).
> 
> Anyway, I'm not arguing any more. I'm telling you that this code will
> 
>  (a) not be pulled this merge window
> 
>  (b) not be pulled EVER unless you clean it up
> 
>  (c) you need to stop with this hackery
> 
> Apparently you knew it was ugly hackery, since you said "I figured
> this would be the most controversial", so it wasn't a surprise to you
> that I'm complaining.

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.

> 
> Why did you send it then? Because I'm serious - this has been a
> pattern, and I  AM DONE.

Because I didn't feel it was as hackery as you do. I'm not even doing
this work alone. I even talk with the memory management folks. They are
not telling me it's hackery.

> 
> Next time, I won't bother to explain why I'm not pulling, because
> apparently even when I explain, you just start making excuses and
> making more garbage up.
> 
> Next time, you're going in the spam filter.
> 
> Because it's annoying that I have to go through your pull requests so
> carefully, but what makes me truly fed up is that this KEEPS
> HAPPENING, and then when I complain you just start arguing about it
> even though you seem to have been aware that the code was shit.

I'm sorry that you feel this way, perhaps I should quit and find
another profession :-p

-- Steve