lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHk-=wh3VnL5Rpuh7tCitOKYDPqWoucFNCh6b3-JR6qZtxCiCw@mail.gmail.com>
Date: Thu, 27 Mar 2025 20:19:09 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Feng Yang <yangfeng@...inos.cn>, 
	Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>
Subject: Re: [GIT PULL] ring-buffer: Updates for v6.15

On Thu, 27 Mar 2025 at 20:01, Steven Rostedt <rostedt@...dmis.org> 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ