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

[Linus Torvalds]

On Thu, 27 Mar 2025 at 20:01, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> Are you OK with the idea of moving the mapping code into the ring
> buffer code (which I think is cleaner) and then keeping track of that
> to get to the pages with the helper function?
>
> struct page *rb_get_page(struct trace_buffer *buffer, unsigned long addr)
> {
>         if (buffer->flags & RB_FL_PHYSICAL) {
>                 addr -= buffer->vmap_start;
>                 addr += buffer->phys_start;
>                 return pfn_to_page(addr >> PAGE_SHIFT);
>         }
>         return virt_to_page(addr);
> }

I think that would have been *enormously* better, yes, if only because
it clearly abstracts things out, and doesn't do the whole "oh, let's
ask what the VM thinks this page is" which really drove me up the
wall.

It may still be based on kernel virtual addresses (and that
"virt_to_page()" looks odd - you can't give it an 'unsigned long', so
I assume this was untested pseudo-code), but now that code at least
makes it very clear that it knows what the addresses are supposed to
be.

And it also makes it clear that both cases are actually based on
underlying contiguous physical addresses just using different virtual
mapping strategies. So now the "page++" thing I complained about is
suddenly not crazy code any more.

Plus the RB_FL_PHYSICAL thing is not just clearer semantically, it
happens to be about a million times faster than doing
"vmalloc_to_page()" (ok, that's obvious hyperbole, but
vmalloc_to_page() kind of a pig and looks things up in the page tables
because that's what it is designed for).

So yes. The kernel virtual address scheme can probably be salvaged.
But it needs doing things *right*, not hackery.

            Linus