On 2025-04-02 10:49, Steven Rostedt wrote:

Now that I learned that the memory passed back from reserve_mem is part
of the memory allocator and just "reserved" and the memory is already
virtually mapped, it can simply use phys_to_virt() on the physical memory
that is returned to get the virtual mapping for that memory!
   (Thanks Mike!)

That makes things much easier, especially since it means that the memory
returned by reserve_mem is no different than the memory retrieved by
page_alloc(). This allows that memory to be memory mapped to user space
no differently than it is mapped by the normal buffer.

This new series does the following:

So I've been loosely following this patch series, and I'm confused about
one thing.

AFAIU one goal is to have the ftrace persistent ring buffer written to
through a memory range returned by vmap_page_range(), and userspace maps
the buffer with virtual mappings.

With respect to architectures with aliasing dcache, is the plan:

A) To make sure all persistent ring buffer mappings are aligned on
   SHMLBA:

Quoting "Documentation/core-api/cachetlb.rst":

  Is your port susceptible to virtual aliasing in its D-cache?
  Well, if your D-cache is virtually indexed, is larger in size than
  PAGE_SIZE, and does not prevent multiple cache lines for the same
  physical address from existing at once, you have this problem.

  If your D-cache has this problem, first define asm/shmparam.h SHMLBA
  properly, it should essentially be the size of your virtually
  addressed D-cache (or if the size is variable, the largest possible
  size).  This setting will force the SYSv IPC layer to only allow user
  processes to mmap shared memory at address which are a multiple of
  this value.

or

B) to flush both the kernel and userspace mappings when a ring buffer
   page is handed over from writer to reader ?

I've seen both approaches being discussed in the recent threads, with
some consensus aiming towards (A), but then the code that follows takes
approach (B).

AFAIU, it we are aiming for approach (A), then I'm missing where
vmap_page_range() guarantees that the _kernel_ virtual mapping is
SHMLBA aligned. AFAIU, only user mappings are aligned on SHMLBA.

And if we aiming towards approach (A), then the explicit flushing
is not needed when handing over pages from writer to reader, but
an SHMLBA alignment should be introduced.

Please let me know if I'm missing something,

Thanks,

Mathieu


- Enforce the memory mapping is page aligned (both the address and the
   size). If not, it errors out.

- Use phys_to_virt() to get to the virtual memory from the reserve_mem
   returned addresses. As the memory is already freed via
   reserve_mem_release_by_name() and it's not mapped by vmap() anymore,
   the free ring buffer code doesn't need to do anything special for
   this mapping.

- Treat the buffer allocated via memmap differently. It still needs to
   be virtually mapped (cannot use phys_to_virt) and it must not be
   freed nor memory mapped to user space. A new flag is added when a buffer
   is created this way to prevent it from ever being memory mapped to user
   space and the ref count is upped so that it can never be freed.

- Use vmap_page_range() instead of using kmalloc_array() to create an array
   of struct pages for vmap().

- Use flush_kernel_vmap_range() instead of flush_dcache_folio()

Changes since v5: 
https://lore.kernel.org/linux-trace-kernel/[email protected]/

- Use %pa instead of %lx for start and size sizes (Mike Rapoport)

- Updated change log to use memblock_alloc() instead of memblock_reserve()
   (Mike Rapoport)

Steven Rostedt (4):
       tracing: Enforce the persistent ring buffer to be page aligned
       tracing: Have reserve_mem use phys_to_virt() and separate from memmap 
buffer
       tracing: Use vmap_page_range() to map memmap ring buffer
       ring-buffer: Use flush_kernel_vmap_range() over flush_dcache_folio()

----
  Documentation/admin-guide/kernel-parameters.txt |  2 +
  Documentation/trace/debugging.rst               |  2 +
  kernel/trace/ring_buffer.c                      |  5 +-
  kernel/trace/trace.c                            | 66 ++++++++++++++++---------
  kernel/trace/trace.h                            |  1 +
  5 files changed, 50 insertions(+), 26 deletions(-)


--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Reply via email to