* Re: [PATCH 17/30] mm: prefer vma_[start,end]_pgoff() to vma->vm_pgoff in kernel/
From: Gregory Price @ 2026-07-09 15:49 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Pedro Falcato, Andrew Morton, Russell King, Dinh Nguyen,
Simon Schuster, James E . J . Bottomley, Helge Deller,
Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Ian Abbott, H Hartley Sweeten, Lucas Stach,
David Airlie, Simona Vetter, Patrik Jakobsson, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Clark, Dmitry Baryshkov,
Tomi Valkeinen, Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Rik van Riel, Harry Yoo,
Jann Horn
In-Reply-To: <akZGqclqQ6gS12Vv@lucifer>
On Thu, Jul 02, 2026 at 12:30:59PM +0100, Lorenzo Stoakes wrote:
>
...
> static inline unsigned long vma_offset(const struct vm_area_struct *vma,
> const unsigned long address)
> {
> /* Retains page offset and tags. */
> return address - vma->vm_start;
> }
>
...
> And I'm not sure it's really all that useful. Perhaps retaining vma_offset()
> would be though.
>
Silly question:
What's the purpose of retaining tags in a non-address value?
That sounds like there's fragility just waiting to be broken.
(I presume you are talking about things like ARM MTE and such, right?)
> This is one that I think makes more sense.
>
> But in general, I'd rather hold off from yet more churn here.
>
> I'm making these changes to establish a basis for virtual page offsets
> introduced in [0], rather than just cleaning up in general.
>
I agree with this. If the refactors here suddenly have to think about
corner cases on things like tags, that's better resolved separately.
~Gregory
^ permalink raw reply
* Re: [PATCH 17/30] mm: prefer vma_[start,end]_pgoff() to vma->vm_pgoff in kernel/
From: Gregory Price @ 2026-07-09 15:49 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <ea87349d63205bf4c26ea79854f179a9bf8cfb0b.1782735110.git.ljs@kernel.org>
On Mon, Jun 29, 2026 at 01:23:28PM +0100, Lorenzo Stoakes wrote:
> Be consistent in using vma_start_pgoff() and vma_end_pgoff(), which clearly
> indicates which part of the VMA the page offset refers to and aids
> greppability.
>
> This is part of a broader series laying the ground to provide a virtual
> page offset for MAP_PRIVATE-file backed anon folios.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Gregory Price <gourry@gourry.net>
^ permalink raw reply
* Re: [PATCH 18/30] mm/vma: remove duplicative vma_pgoff_offset() helper
From: Gregory Price @ 2026-07-09 15:50 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <10671c2fc5d0dd4e3bf497181923e63e46053df1.1782735110.git.ljs@kernel.org>
On Mon, Jun 29, 2026 at 01:23:29PM +0100, Lorenzo Stoakes wrote:
> This is doing what linear_page_index() does, so eliminate it and replace it
> with linear_page_index().
>
> Update the VMA userland tests to reflect this change.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Gregory Price <gourry@gourry.net>
^ permalink raw reply
* Re: [PATCH v3 14/17] verification/rvgen: Add selftests for rvgen kunit
From: Gabriele Monaco @ 2026-07-09 15:52 UTC (permalink / raw)
To: Nam Cao, linux-trace-kernel, linux-kernel
Cc: Thomas Weissschuh, Tomas Glozar, John Kacur, Wen Yang,
Steven Rostedt
In-Reply-To: <87a4s0fuqx.fsf@yellow.woof>
On Thu, 2026-07-09 at 10:11 +0200, Nam Cao wrote:
> Gabriele Monaco <gmonaco@redhat.com> writes:
> > The rvgen kunit command patches monitor files and adds necessary
> > definitions for kunit tests.
> >
> > Add a test case validating its behaviour on dummy generated files and
> > comparing it against reference files, like it's done for rvgen monitor.
> >
> > Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
>
> I am not entirely sure about this. In the future, when the generation
> output is changed, all the references files also need to be updated. So
> it's not clear to me if this really saves future development effort.
Yeah that's an issue with all this testing with golden files. At least unless we
change the output drastically, changes to those should be easy to apply and to
review (and LLMs can help here too).
> But well, we will see. For now:
>
> Reviewed-by: Nam Cao <namcao@linutronix.de>
Thanks,
Gabriele
^ permalink raw reply
* Re: [PATCH 19/30] mm: use linear_page_[index, delta]() consistently
From: Gregory Price @ 2026-07-09 15:55 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <bf56e2e98b512962a2fb88900d535a0e9e6769d8.1782735110.git.ljs@kernel.org>
On Mon, Jun 29, 2026 at 01:23:30PM +0100, Lorenzo Stoakes wrote:
> There are a number of places where we open code what linear_page_index()
> and linear_page_delta() calculate.
>
> Replace this code with the appropriate functions for consistency.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
one nit
Reviewed-by: Gregory Price <gourry@gourry.net>
...
> diff --git a/drivers/comedi/comedi_fops.c b/drivers/comedi/comedi_fops.c
> index c09bbe04be6c..536c25d8dcee 100644
> --- a/drivers/comedi/comedi_fops.c
> +++ b/drivers/comedi/comedi_fops.c
> @@ -25,6 +25,7 @@
> #include <linux/fs.h>
> #include <linux/comedi/comedidev.h>
> #include <linux/cdev.h>
> +#include <linux/pagemap.h>
>
> #include <linux/io.h>
> #include <linux/uaccess.h>
> @@ -2462,7 +2463,7 @@ static int comedi_vm_access(struct vm_area_struct *vma, unsigned long addr,
> {
> struct comedi_buf_map *bm = vma->vm_private_data;
> unsigned long offset =
> - addr - vma->vm_start + (vma->vm_pgoff << PAGE_SHIFT);
> + addr - vma->vm_start + (vma_start_pgoff(vma) << PAGE_SHIFT);
>
Obviously correct, but was this intended for a different patch?
~Gregory
^ permalink raw reply
* Re: [PATCH 19/30] mm: use linear_page_[index, delta]() consistently
From: Gregory Price @ 2026-07-09 15:57 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <ak_EivwcDDdn1Xvp@gourry-fedora-PF4VCD3F>
On Thu, Jul 09, 2026 at 11:55:54AM -0400, Gregory Price wrote:
> > @@ -2462,7 +2463,7 @@ static int comedi_vm_access(struct vm_area_struct *vma, unsigned long addr,
> > {
> > struct comedi_buf_map *bm = vma->vm_private_data;
> > unsigned long offset =
> > - addr - vma->vm_start + (vma->vm_pgoff << PAGE_SHIFT);
> > + addr - vma->vm_start + (vma_start_pgoff(vma) << PAGE_SHIFT);
> >
>
> Obviously correct, but was this intended for a different patch?
>
> ~Gregory
bleh already caught, sorry for the noise :]
^ permalink raw reply
* Re: [PATCH v3 06/11] mm/cma: Allow dynamically creating CMA areas
From: Thierry Reding @ 2026-07-09 15:59 UTC (permalink / raw)
To: Marek Szyprowski
Cc: David Hildenbrand (Arm), Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Hunter, Mikko Perttunen, Yury Norov,
Rasmus Villemoes, Russell King, Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Robin Murphy, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Catalin Marinas, Will Deacon, devicetree,
linux-tegra, linux-kernel, dri-devel, linux-media,
linux-arm-kernel, linux-s390, linux-mm, iommu, linaro-mm-sig,
linux-trace-kernel
In-Reply-To: <83e5e74d-7106-4e14-9d10-56438372f6a3@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 1925 bytes --]
On Thu, Jul 09, 2026 at 07:56:45AM +0200, Marek Szyprowski wrote:
> On 08.07.2026 10:35, David Hildenbrand (Arm) wrote:
> > On 7/7/26 12:02, Marek Szyprowski wrote:
> >> On 01.07.2026 18:08, Thierry Reding wrote:
> >>> From: Thierry Reding <treding@nvidia.com>
> >>>
> >>> There is no technical reason why there should be a limited number of CMA
> >>> regions, so extract some code into helpers and use them to create extra
> >>> functions (cma_create() and cma_free()) that allow creating and freeing,
> >>> respectively, CMA regions dynamically at runtime.
> >>
> >> Well, the technical reason for not creating cma regions dynamically at
> >> runtime is that on some architectures (like 32bit ARM) the early fixup
> >> for the region is needed to make it functional for DMA.
> > Can you point me at the code that does that? Thanks!
> Check dma_contiguous_early_fixup() and dma_contiguous_remap() in
> arch/arm/mm/dma-mapping.c. Those functions ensures that the CPU mappings for
> the CMA reserved region in linear map are remapped with 4k pages instead
> of the 1M sections, so later, it will be possible to alter the mappings and
> change them to coherent when needed (altering 1M sections is not possible,
> because each process has it's own level-1 array even for the kernel linear
> mapping).
>
>
>
> However, in the use case in this patchset the reserved region is only shared
> with buddy allocator by using the CMA infrastructure, not registered to the
> regular DMA-mapping API, so it would work fine. I'm not convinced that this
> is the right API to use for this though.
Are you saying you're not convinced that CMA is the right API to use for
this? Or something else?
I certainly don't think we want to get the DMA-mapping API involved for
this because that always implies that we perform cache operations, which
we specifically don't want for this memory.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 0/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
From: Vincent Donnefort @ 2026-07-09 16:00 UTC (permalink / raw)
To: rostedt, mhiramat, linux-trace-kernel
Cc: mathieu.desnoyers, kernel-team, linux-kernel, Vincent Donnefort
This series addresses a few issues in the trace remote buffers allocation
and descriptor handling, mostly reported by Sashiko bot on top of v1.
Changelog:
v1 -> v2:
- Leak fix: increment nr_cpus early (after meta_va allocation) instead of
in the error path.
- Added struct_len double-counting fix.
- Added sparse CPU masks support in ring_buffer_desc().
v1: https://lore.kernel.org/all/20260708133201.295072-1-vdonnefort@google.com/
Vincent Donnefort (3):
tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
tracing/remotes: Fix struct_len in trace_remote_alloc_buffer()
ring-buffer: Allow sparse CPU masks in ring_buffer_desc()
kernel/trace/ring_buffer.c | 5 +----
kernel/trace/trace_remote.c | 18 ++++++------------
2 files changed, 7 insertions(+), 16 deletions(-)
base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply
* [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
From: Vincent Donnefort @ 2026-07-09 16:00 UTC (permalink / raw)
To: rostedt, mhiramat, linux-trace-kernel
Cc: mathieu.desnoyers, kernel-team, linux-kernel, Vincent Donnefort,
Sashiko
In-Reply-To: <20260709160017.1729517-1-vdonnefort@google.com>
If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus
is not yet incremented for the current CPU. As a consequence, on error,
half-allocated rb_desc will not be freed in trace_remote_free_buffer().
Increment desc->nr_cpus as soon as the first allocation for the current
CPU has succeeded.
Fixes: 96e43537af54 ("tracing: Introduce trace remotes")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
index 2a6cc000ec98..d48042239d58 100644
--- a/kernel/trace/trace_remote.c
+++ b/kernel/trace/trace_remote.c
@@ -1006,6 +1006,8 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size,
if (!rb_desc->meta_va)
goto err;
+ desc->nr_cpus++;
+
for (id = 0; id < nr_pages; id++) {
rb_desc->page_va[id] = (unsigned long)__get_free_page(GFP_KERNEL);
if (!rb_desc->page_va[id])
@@ -1013,7 +1015,6 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size,
rb_desc->nr_page_va++;
}
- desc->nr_cpus++;
desc->struct_len += offsetof(struct ring_buffer_desc, page_va);
desc->struct_len += struct_size(rb_desc, page_va, rb_desc->nr_page_va);
rb_desc = __next_ring_buffer_desc(rb_desc);
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 2/3] tracing/remotes: Fix struct_len in trace_remote_alloc_buffer()
From: Vincent Donnefort @ 2026-07-09 16:00 UTC (permalink / raw)
To: rostedt, mhiramat, linux-trace-kernel
Cc: mathieu.desnoyers, kernel-team, linux-kernel, Vincent Donnefort,
Sashiko
In-Reply-To: <20260709160017.1729517-1-vdonnefort@google.com>
Pre-calculate desc->struct_len up-front in trace_remote_alloc_buffer()
with trace_buffer_desc_size() to fix double-counting.
While at it, use the accessor __first_ring_buffer_desc().
Fixes: 96e43537af54 ("tracing: Introduce trace remotes")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
index d48042239d58..0f6ef5c36d84 100644
--- a/kernel/trace/trace_remote.c
+++ b/kernel/trace/trace_remote.c
@@ -979,27 +979,22 @@ EXPORT_SYMBOL_GPL(trace_remote_free_buffer);
int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size, size_t buffer_size,
const struct cpumask *cpumask)
{
+ size_t min_desc_size = trace_buffer_desc_size(buffer_size, cpumask_weight(cpumask));
unsigned int nr_pages = max(DIV_ROUND_UP(buffer_size, PAGE_SIZE), 2UL) + 1;
- void *desc_end = desc + desc_size;
struct ring_buffer_desc *rb_desc;
int cpu, ret = -ENOMEM;
- if (desc_size < struct_size(desc, __data, 0))
+ if (desc_size < min_desc_size)
return -EINVAL;
desc->nr_cpus = 0;
- desc->struct_len = struct_size(desc, __data, 0);
+ desc->struct_len = min_desc_size;
- rb_desc = (struct ring_buffer_desc *)&desc->__data[0];
+ rb_desc = __first_ring_buffer_desc(desc);
for_each_cpu(cpu, cpumask) {
unsigned int id;
- if ((void *)rb_desc + struct_size(rb_desc, page_va, nr_pages) > desc_end) {
- ret = -EINVAL;
- goto err;
- }
-
rb_desc->cpu = cpu;
rb_desc->nr_page_va = 0;
rb_desc->meta_va = (unsigned long)__get_free_page(GFP_KERNEL);
@@ -1015,8 +1010,6 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size,
rb_desc->nr_page_va++;
}
- desc->struct_len += offsetof(struct ring_buffer_desc, page_va);
- desc->struct_len += struct_size(rb_desc, page_va, rb_desc->nr_page_va);
rb_desc = __next_ring_buffer_desc(rb_desc);
}
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 3/3] ring-buffer: Allow sparse CPU masks in ring_buffer_desc()
From: Vincent Donnefort @ 2026-07-09 16:00 UTC (permalink / raw)
To: rostedt, mhiramat, linux-trace-kernel
Cc: mathieu.desnoyers, kernel-team, linux-kernel, Vincent Donnefort,
Sashiko
In-Reply-To: <20260709160017.1729517-1-vdonnefort@google.com>
No user currently relies on sparse CPU masks, but the descriptor logic already
supports them via linear fallback. Remove the arbitrary limitation.
Fixes: 2e67fabd8b77 ("ring-buffer: Introduce ring-buffer remotes")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 56a328e94395..4e7b4ce7b8a7 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2329,10 +2329,7 @@ static struct ring_buffer_desc *ring_buffer_desc(struct trace_buffer_desc *trace
size_t len;
int i;
- if (!trace_desc)
- return NULL;
-
- if (cpu >= trace_desc->nr_cpus)
+ if (!trace_desc || !trace_desc->nr_cpus)
return NULL;
end = (struct ring_buffer_desc *)((void *)trace_desc + trace_desc->struct_len);
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* Re: [PATCH v2 1/4] mm/migrate: do not migrate folios mapped into VM_LOCKED VMAs under compaction
From: Zi Yan @ 2026-07-09 16:10 UTC (permalink / raw)
To: Wandun
Cc: Lorenzo Stoakes, vbabka, david, rostedt, mhiramat,
Alexander.Krabler, hughd, fvdl, bigeasy, linux-mm, linux-kernel,
linux-trace-kernel, linux-rt-devel, akpm, surenb, mhocko,
jackmanb, hannes, riel, liam, harry, jannh, lance.yang,
mathieu.desnoyers, matthew.brost, joshua.hahnjy, rakie.kim,
byungchul, gourry, ying.huang, apopple, pfalcato
In-Reply-To: <c372e68f-2bfc-45b6-a431-01bf55717247@gmail.com>
On 8 Jul 2026, at 23:31, Wandun wrote:
> On 7/7/26 21:44, Lorenzo Stoakes wrote:
>> (Being really nitty, your subject line is too long)
>>
>> Please don't reference legacy VMA flags for newer patches 'do not migrate
>> folios mapped into mlocked VMAs...' works just as well.
> Got it.
>>
>> On Tue, Jul 07, 2026 at 08:59:22PM +0800, Wandun Chen wrote:
>>> From: Wandun Chen <chenwandun@lixiang.com>
>>>
>>> When compact_unevictable_allowed=0, unevictable pages should not be
>>> migrated. However, mlock_folio_batch in the mlock[all] syscall introduces
>>> a race, mlock_folio() sets PG_mlocked immediately but defers PG_unevictable
>>> to mlock_folio_batch(), causing pages that are about to become unevictable
>>> to be migrated, which violates the intent of compact_unevictable_allowed,
>>> and causes spike latency in RT kernels [1].
>>>
>>> In order to fix this, migration is forbidden for pages mapped into VMAs
>>> marked with VM_LOCKED. In addition, two early-return paths are introduced,
>>
>> Please don't reference legacy VMA flags. -> VMA_LOCKED_BIT.
> Got it.
>>
>>> filter out mlocked pages, return early to avoid unnecessary operations.
>>>
>>> Fixes: 90d07210ab55 ("mm: mlock: use folios and a folio batch internally")
>>
>> Hmmmm why do you think my patch caused this? That was just a folio conversion?
>
> Oh, I made a mistake, your patch was just a folio conversion.
>
> Batching mlocked page was introduced in v5.18 by:
> commit 2fbb0c10d1e8 ("mm/munlock: mlock_page() munlock_page() batch by pagevec")
>
> Setting sysctl_compact_unevictable_allowed to zero was introduced in v5.7 by:
> commit 6923aa0d8c62 ("mm/compaction: Disable compact_unevictable_allowed on RT")
>
> So this issue exist after v5.18.
>
>>
>> Also I didn't think we liked having fixes spotted about a series with non-fixes
>> tags?
> Got it, will split this series in next version.
>>
>>> Reported-by: Alexander Krabler <Alexander.Krabler@kuka.com>
>>> Closes: https://lore.kernel.org/all/DU0PR01MB10385345F7153F334100981888259A@DU0PR01MB10385.eurprd01.prod.exchangelabs.com/ [1]
>>> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
>>> Signed-off-by: Wandun Chen <chenwandun@lixiang.com>
>>> Link: https://lore.kernel.org/linux-rt-users/33275585-f2db-4779-89f0-3ae24b455a67@suse.cz/#t
>>> ---
>>> include/linux/compaction.h | 6 ++++++
>>> include/linux/rmap.h | 3 +++
>>> mm/compaction.c | 8 +++++++-
>>> mm/migrate.c | 23 +++++++++++++++++++----
>>> mm/rmap.c | 12 +++++++++---
>>> 5 files changed, 44 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
>>> index f29ef0653546..04e60f65b976 100644
>>> --- a/include/linux/compaction.h
>>> +++ b/include/linux/compaction.h
>>> @@ -106,6 +106,7 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order,
>>> extern void __meminit kcompactd_run(int nid);
>>> extern void __meminit kcompactd_stop(int nid);
>>> extern void wakeup_kcompactd(pg_data_t *pgdat, int order, int highest_zoneidx);
>>> +extern bool compaction_allow_unevictable(void);
>>
>> Don't use extern. We remove extern as we go it's not needed.
> Got it.
>>
>>>
>>> #else
>>> static inline void reset_isolation_suitable(pg_data_t *pgdat)
>>> @@ -131,6 +132,11 @@ static inline void wakeup_kcompactd(pg_data_t *pgdat,
>>> {
>>> }
>>>
>>> +static inline bool compaction_allow_unevictable(void)
>>> +{
>>> + return true;
>>> +}
>>> +
>>> #endif /* CONFIG_COMPACTION */
>>>
>>> struct node;
>>> diff --git a/include/linux/rmap.h b/include/linux/rmap.h
>>> index 8dc0871e5f00..359c7426b6b9 100644
>>> --- a/include/linux/rmap.h
>>> +++ b/include/linux/rmap.h
>>> @@ -102,6 +102,9 @@ enum ttu_flags {
>>> * do a final flush if necessary */
>>> TTU_RMAP_LOCKED = 0x80, /* do not grab rmap lock:
>>> * caller holds it */
>>> + TTU_RESPECT_MLOCK = 0x100,/* leave VM_LOCKED vmas mapped instead
>>
>> -> VMA_LOCKED_BIT please. Also maybe just say mlock'd?
> Got it.
>>
>>> + * of installing a migration entry
>>> + */
>>> };
>>>
>>> #ifdef CONFIG_MMU
>>> diff --git a/mm/compaction.c b/mm/compaction.c
>>> index f08765ade014..5d256930e389 100644
>>> --- a/mm/compaction.c
>>> +++ b/mm/compaction.c
>>> @@ -1116,7 +1116,8 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>>> is_unevictable = folio_test_unevictable(folio);
>>>
>>> /* Compaction might skip unevictable pages but CMA takes them */
>>> - if (!(mode & ISOLATE_UNEVICTABLE) && is_unevictable)
>>> + if (!(mode & ISOLATE_UNEVICTABLE) &&
>>> + (is_unevictable || folio_test_mlocked(folio)))
>>
>> Maybe just change is_unevictable to include this check?
>>
>> Like:
>>
>> is_unevictable = folio_test_unevictable(folio) ||
>> folio_test_mlocked(folio);
>>
>> ?
> Sounds good, I'll fold mlock into is_unevictable so both checks
> stay consistent.
>>
>> Also later you have:
>>
>> if (((mode & ISOLATE_ASYNC_MIGRATE) && is_dirty) ||
>> (mapping && is_unevictable)) {
>> ...
>>
>> Which doesn't account for mlock as-is? Is that correct?
> IIUC, The is_unevictable check here mainly serves as a cheap pre-filter
> for inaccessible mappings (which are always unevictable folio).
> It's unrelated to the mlock.
>
>>
>>
>>
>>> goto isolate_fail_put;
>>>
>>> /*
>>> @@ -1898,6 +1899,11 @@ typedef enum {
>>> * compactable pages.
>>> */
>>> static int sysctl_compact_unevictable_allowed __read_mostly = CONFIG_COMPACT_UNEVICTABLE_DEFAULT;
>>> +
>>> +bool compaction_allow_unevictable(void)
>>> +{
>>> + return sysctl_compact_unevictable_allowed;
>>> +}
>>
>> You add this helper but isolate_migratepages() still references
>> sysctl_compact_unevictable_allowed directly?
> I'll route that reference through compaction_allow_unevictable()
> in next version.
>>
>>> /*
>>> * Tunable for proactive compaction. It determines how
>>> * aggressively the kernel should compact memory in the
>>> diff --git a/mm/migrate.c b/mm/migrate.c
>>> index a786549551e3..3a15eb13e82b 100644
>>> --- a/mm/migrate.c
>>> +++ b/mm/migrate.c
>>> @@ -1202,7 +1202,7 @@ static void migrate_folio_done(struct folio *src,
>>> static int migrate_folio_unmap(new_folio_t get_new_folio,
>>> free_folio_t put_new_folio, unsigned long private,
>>> struct folio *src, struct folio **dstp, enum migrate_mode mode,
>>> - struct list_head *ret)
>>> + struct list_head *ret, enum migrate_reason reason)
>>> {
>>> struct folio *dst;
>>> int rc = -EAGAIN;
>>> @@ -1210,6 +1210,7 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
>>> struct anon_vma *anon_vma = NULL;
>>> bool locked = false;
>>> bool dst_locked = false;
>>> + enum ttu_flags ttu = 0;
>>
>> You reference ttu only in an if-block below no? So why are you declaring
>> this here? Move it to the if-block.
> Got it. I'll move it to the if-block in next version.
>>
>>>
>>> dst = get_new_folio(src, private);
>>> if (!dst)
>>> @@ -1249,9 +1250,15 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
>>> folio_lock(src);
>>> }
>>> locked = true;
>>> - if (folio_test_mlocked(src))
>>> + if (folio_test_mlocked(src)) {
>>> old_folio_state |= FOLIO_WAS_MLOCKED;
>>>
>>> + if (reason == MR_COMPACTION && !compaction_allow_unevictable()) {
>>
>> This should really be a helper since you repeat yourself and it's not
>> obvious what this is checking.
>>
>> Like:
>>
>> static migrate_mlock_allowed(enum migrate_reason reason)
>> {
>> /* Only compaction is disallowed. */
>> if (reason != MR_COMPACTION)
>> return true;
>>
>> /* If we can compact unevictable folios, we are ok. */
>> if (compaction_allow_unevictable())
>> return true;
>>
>> /* Conservative: if any folio could be mlock()'d, disallow. */
>> return false;
>> }
>>
>> Then you could self-document what you're checking and avoid code duplication below.
> Got it, it is more clear, thanks.
>>
>>> + rc = -EBUSY;
>>> + goto out;
>>> + }
>>> + }
>>> +
>>> if (folio_test_writeback(src)) {
>>> /*
>>> * Only in the case of a full synchronous migration is it
>>> @@ -1324,7 +1331,14 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
>>> /* Establish migration ptes */
>>> VM_BUG_ON_FOLIO(folio_test_anon(src) &&
>>> !folio_test_ksm(src) && !anon_vma, src);
>>
>> Useful to convert VM_BUG_*() -> VM_WARN_*() (possibly _ONCE() here also) as we go!
> Got it.
>>
>>> - try_to_migrate(src, mode == MIGRATE_ASYNC ? TTU_BATCH_FLUSH : 0);
>>> +
>>> + if (mode == MIGRATE_ASYNC)
>>> + ttu |= TTU_BATCH_FLUSH;
>>> +
>>> + if (reason == MR_COMPACTION && !compaction_allow_unevictable())
>>
>> See above about deduplicating.
>>
>>> + ttu |= TTU_RESPECT_MLOCK;
>>
>> Hmm. I don't love 'respect mlock'. I guess we only know about the reason
>> being compaction here.
> Right, we only know about the reason.
>>
>> But I'm confused anyway. We have the folio, why aren't we just checking for
>> PG_mlocked() here instead of getting the rmap to see if it's mapped
>> anywhere with VMA_LOCKED_BIT?
>
> There was a race scenario without patch 02, vma may already marked with
> VMA_LOCKED_BIT but folio has't marked with mlocked, such as below:
>
>
> CPUA: mlock() CPUB: compaction / migration
>
> mmap_write_lock()
> mlock_fixup set VM_CLOKED
> mlock_pte_range
> mlock_folio(page N)
>
> isolate page N+50 (mlock hasn't reached it)
> migrate_folio_unmap
> folio_test_mlocked() --> false, but VMA:VM_LOCKED
> try_to_migrate()
> rmap_walk(P) [anon_vma rwsem / i_mmap_rwsem, read]
> try_to_migrate_one -->install migration entry
> ...reaches page N+50
> skip migration entry (without patch 02)
> mmap_write_unlock()
>
>
> access page N + 50 --> wait for migration complete
>
> migrate_folios_move
> ....
> migrate complete
>
>
>
> If apply patch 02, mlock itself will wait migration complete, so checking
> VMA_LOCKED_BIT in the rmap path is no longer necessary, but this logic is
> retained to avoid unnecessary migration operations.
>
For this race condition, since VMA has VM_LOCKED_BIT set, maybe you can
extend rwc->invalid_vma semantics to return 1:SKIP, 0:OK, -1:STOP.
Then install an invalid_vma function when !compaction_allow_unevictable().
The invalid_vma function returns -1:STOP for any vma with VM_LOCKED_BIT set.
__rmap_walk_file() and rmap_walk_anon() will need to break when -1:STOP
is returned.
This approach will not require a new TTU flag, although it requires more
code changes.
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH v3 04/11] arm64/mm: Add set_memory_device() and set_memory_normal()
From: Thierry Reding @ 2026-07-09 16:13 UTC (permalink / raw)
To: Will Deacon
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Hunter,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Sowjanya Komatineni, Luca Ceresoli,
Mikko Perttunen, Yury Norov, Rasmus Villemoes, Russell King,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Marek Szyprowski, Robin Murphy, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Catalin Marinas, Thierry Reding, devicetree, linux-tegra,
linux-kernel, dri-devel, linux-media, linux-arm-kernel,
linux-s390, linux-mm, iommu, linaro-mm-sig, linux-trace-kernel,
Thierry Reding, Chun Ng
In-Reply-To: <ak5CXzGStC6ZmtwI@orome>
[-- Attachment #1: Type: text/plain, Size: 7996 bytes --]
On Wed, Jul 08, 2026 at 02:50:04PM +0200, Thierry Reding wrote:
> On Tue, Jul 07, 2026 at 12:27:13PM +0100, Will Deacon wrote:
> > On Mon, Jul 06, 2026 at 03:49:24PM +0200, Thierry Reding wrote:
> > > On Fri, Jul 03, 2026 at 06:13:31PM +0100, Will Deacon wrote:
> > > > On Thu, Jul 02, 2026 at 06:41:23PM +0200, Thierry Reding wrote:
> > > > > On Thu, Jul 02, 2026 at 03:46:44PM +0200, Thierry Reding wrote:
> > > > > > On Thu, Jul 02, 2026 at 10:18:47AM +0100, Will Deacon wrote:
> > > > > > > On Wed, Jul 01, 2026 at 06:08:15PM +0200, Thierry Reding wrote:
> > > > > > > > From: Chun Ng <chunn@nvidia.com>
> > > > > > > >
> > > > > > > > Add helpers to swap PROT_NORMAL and PROT_DEVICE_nGnRnE protection bits
> > > > > > > > on a kernel-linear-map range.
> > > > > > >
> > > > > > > That sounds like a really terrible idea. Why is this necessary and how
> > > > > > > does it interact with things like load_unaligned_zeropad()?
> > > > > >
> > > > > > This is necessary because once the memory controller has walled off the
> > > > > > new memory region the CPU must not access it under any circumstances or
> > > > > > it'll cause the CPU to lock up (I think technically it'll hit an SError
> > > > > > but in practice that just means it'll freeze, as far as I can tell).
> > > > > >
> > > > > > Probably doesn't interact well at all with load_unaligned_zeropad().
> > > > > >
> > > > > > > I think you should unmap the memory from the linear map and memremap()
> > > > > > > it instead.
> > > > > >
> > > > > > Given that the memory can never be accessed by the CPU after the memory
> > > > > > controller locks it down, I don't think we'll even need memremap(). The
> > > > > > only thing we really need is the sg_table we hand out via the DMA BUFs
> > > > > > so that they can be used by device drivers to program their DMA engines
> > > > > > internally.
> > > > > >
> > > > > > Looking through some of the architecture code around this, shouldn't we
> > > > > > simply be using set_memory_encrypted() and set_memory_decrypted() for
> > > > > > this? While they might've been created for slightly other use-cases,
> > > > > > they seem to be doing exactly what we want (i.e. remove the page range
> > > > > > from the linear mapping and flushing it, or restoring the valid bit and
> > > > > > standard permissions, respectively).
> > > > >
> > > > > Ah... I guess we can't do it because we're not in a realm world and so
> > > > > the early checks in __set_memory_enc_dec() would return early and turn
> > > > > it into a no-op.
> > > > >
> > > > > How about if I extract a common helper and provide set_memory_p() and
> > > > > set_memory_np() in terms of those. Those are available on x86 and
> > > > > PowerPC as well, so fairly standard. I suppose at that point we're
> > > > > closer to set_memory_valid().
> > > >
> > > > Why not just call set_direct_map_invalid_noflush() +
> > > > flush_tlb_kernel_range() for each page? We already have APIs for this.
> > >
> > > Having a "standard" helper with a fixed and documented purposed seemed
> > > like a preferable approach for this particular case. We also may want to
> > > make the driver that uses this buildable as a module, in which case we'd
> > > need to export these rather low-level APIs. And then there's also the
> > > fact that we typically call this on a rather large region of memory
> > > (usually something like 512 MiB), so doing it page-by-page is rather
> > > suboptimal.
> > >
> > > > The big challenge I see with any linear map manipulation, however, is
> > > > that it will rely on can_set_direct_map() which likely means you need to
> > > > give up some performance and/or security to make this work. Does memory
> > > > become inaccesible dynamically at runtime? If not, the best bet would
> > > > be to describe it as a carveout in the DT and mark it as "no-map" so
> > > > we avoid mapping it in the first place.
> > >
> > > VPR exists in two modes: static and resizable. For static VPR we do
> > > exactly that: describe it as carveout in DT with no-map and deal with it
> > > accordingly in the driver. Resizable VPR is for device that have small
> > > amounts of RAM. Content-protected video playback will in the worst case
> > > consume around 1.8 GiB of RAM, so we want to be able to reuse for other
> > > purposes when VPR is unused on those devices. In that case, the memory
> > > is also described as a reserved-memory region in DT, but it is marked as
> > > reusable so that it can be managed by CMA.
> > >
> > > The resize operation is fairly slow to begin with because we need to
> > > stall the GPU and put it into reset before the operation, then take it
> > > out of reset and resume it afterwards.
> > >
> > > What kind of performance impact do you expect?
> >
> > You'll need to measure it, but we've seen reports of double-digit
> > percentage regressions in performance and power. As I said, the problem
> > is that you need to split the linear map to 4k page at runtime to unmap
> > the dynamic carveout, but that isn't something that can be done on most
> > CPUs. Therefore you end up having to use page-granular mappings for the
> > entire thing, similarly to how 'rodata_full' drives can_set_direct_map()
> > and the perf/power hit affects everything.
> >
> > It's hard to know what to suggest... I wonder if any of the memory
> > hotplug logic could help here?
>
> I've read up on memory hotplug a bit and it sounds like it could fit
> this really nicely. Given that we only use CMA (along with the extra
> patches to it) to make sure that any buffers are reclaimed for VPR use,
> we should be able to get rid of the CMA usage altogether and replace it
> with online_pages() and offline_pages() instead. Rather than using a
> fixed set of CMA areas like we currently do, each "chunk" in the VPR
> driver could represent a memory block instead (which looks like it will
> be 128 MiB for 4 KiB pages and 512 MiB for 64 KiB pages). We currently
> use 512 MiB as the chunk size, so it should be relatively similar and
> easy to adjust.
>
> One issue that we would absolutely need this memory to be ZONE_MOVABLE
> from the start. Using no-map in DT and then online_pages() probably will
> not work because there's no struct page for the memory. So we're left
> with keeping the memory onlined by default, in which case we'd need some
> way for DT to instruct the memory to be put into ZONE_MOVABLE always.
>
> There's a "hotpluggable" property for "memory" nodes, maybe that can be
> extended to apply to reserved-memory nodes as well?
I haven't been having much success with this. memblock_mark_hotplug()
doesn't have much of an effect because the kernel clears this flag
automatically at some point, so by the time the movable zone is created
there's no memory left that's marked hotpluggable. I don't know if it's
a good idea to modify the code to keep the flag.
Another thing I briefly tried was to use add_memory_driver_managed()
together with the no-map flag in an attempt to get the memory explicitly
added as movable, but that fails because __request_resource() notices
that the reserved memory is actually part of the system RAM that was
registered earlier.
I think in order for this to work the bindings would probably need to
change, such that reserved-memory nodes aren't used but it's described
using the memory nodes instead. That way a piece of system RAM could be
carved out and added by the VPR driver. I don't know if the kernel would
like this kind of splicing of the system RAM, though.
It's all very close to what I need for this, but doesn't quite fit. Any
ideas which of the options is best? Right now it sounds like finding a
way to make this region explicitly ZONE_MOVABLE would be the best. That
should allow offline_pages() and online_pages() to be used, which seems
like the cleanest approach.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] rv/reactors: fix lockdep "Invalid wait context" in rv_react()
From: Wen Yang @ 2026-07-09 16:37 UTC (permalink / raw)
To: Thomas Weißschuh, Gabriele Monaco
Cc: Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <20260708174123-e5f98837-3d81-43ca-8ede-bb5f09e8ffba@linutronix.de>
On 7/8/26 23:47, Thomas Weißschuh wrote:
> On Mon, Jul 06, 2026 at 04:02:08PM +0200, Gabriele Monaco wrote:
>> On Tue, 2026-06-23 at 11:38 +0200, Thomas Weißschuh wrote:
>>> This now allows reactors to take (raw) spinlocks. The original idea was
>>> to not allow that as a reactor can be called from LD_WAIT_FREE context.
>>> So I am not sure this is the right fix. Not that I have a better one
>>> available right now.
>>
>> As far as I understand it, LD_WAIT_FREE is fairly impossible to apply on
>> preemptible code (here we see it hit by an interrupt).
>>
>> Since we kind of have to allow raw spinlock to avoid this (even if we don't take
>> them explicitly), why wouldn't a reactor ever be allowed to take raw spinlocks?
>>
>> Technically it wouldn't be wrong to take locks from RV monitors, although most
>> monitors don't do it explicitly.
>> If we ever happen to take the wrong lock explicitly from a reactor triggered by
>> a nasty event (e.g. sched_switch), I believe lockdep would still be complaining
>> down that path, so we probably don't need to be too strict in rv_react().
>>
>> Obviously we don't want to disable interrutps for LD_WAIT_FREE to hold either.
>>
>> Am I missing something?
>
> No, I was just very confused.
> Let's go ahead with this.
>
>
Hi,
How about a context-sensitive approach, eg:
NMI/hardirq (interrupts masked, scheduler cannot run):
Keep LD_WAIT_FREE. The false positive cannot arise in this path,
and the original constraint against raw spinlocks is preserved where
it is meaningful.
task/softirq/PREEMPT_RT irq thread (preemptible):
Raise to LD_WAIT_SPIN. Raw spinlocks become permitted — as Gabriele
note, this is a necessary consequence of any fix in this path.
like this:
+ static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_SPIN);
+ static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map_atomic, LD_WAIT_FREE);
+ struct lockdep_map *map;
...
+ map = (in_nmi() || in_hardirq()) ? &rv_react_map_atomic :
&rv_react_map;
- lock_map_acquire_try(&rv_react_map);
+ lock_map_acquire_try(map);
...
monitor->react(msg, args);
- lock_map_release(&rv_react_map);
+ lock_map_release(map);
--
Best wishes,
Wen
^ permalink raw reply
* Re: [PATCH 3/3] rv/reactors: add KUnit tests for reactor_panic
From: Wen Yang @ 2026-07-09 16:46 UTC (permalink / raw)
To: Gabriele Monaco; +Cc: Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <c8c7568dd9293b0a718584c1aa193692c128d77d.camel@redhat.com>
On 7/6/26 23:00, Gabriele Monaco wrote:
> On Tue, 2026-06-16 at 00:44 +0800, wen.yang@linux.dev wrote:
>> +/*
>> + * Test 2: panic notifier chain is reachable.
>> + *
>> + * vpanic() calls atomic_notifier_call_chain(&panic_notifier_list, ...).
>> + * Drive the chain directly to verify panic notifiers receive the
>> notification —
>> + * the observable side-effect of reactor_panic without halting the system.
>> + */
>> +static void test_panic_notifier_called(struct kunit *test)
>> +{
>> + atomic_notifier_chain_register(&panic_notifier_list, &mock_panic_nb);
>> + atomic_notifier_call_chain(&panic_notifier_list, 0,
>> + "panic violation message");
>> + atomic_notifier_chain_unregister(&panic_notifier_list,
>> &mock_panic_nb);
>
> I just realised this isn't even testing the reactor, it's testing the
> notifier_chain thing, I don't think we really need this here or am I missing
> something?
>
Thanks,
Since rv_panic_reaction calls vpanic() which is __noreturn, direct
testing is not feasible in KUnit.
A preparatory v2 drops reactor_printk_kunit.c and reactor_panic_kunit.c
entirely, replacing them with a single rv_reactors_kunit.c containing
two suites:
rv_reactor_registration: register/unregister lifecycle, duplicate
rejection (-EINVAL), name-too-long rejection, and safe unregister
of a never-registered reactor.
rv_react_dispatch: null-callback guard, callback invocation check,
and the mdelay lockdep stress test.
v2 also adds EXPORT_SYMBOL_GPL for rv_react(), rv_register_reactor(),
and rv_unregister_reactor(), and the Kconfig entry is tristate.
Additional fix: rv_unregister_reactor()
Testing exposed a real bug: rv_unregister_reactor() called list_del()
unconditionally. On a never-registered reactor the list_head is
zero-initialised, so list_del() dereferences NULL->prev and crashes.
v2 adds a patch that iterates rv_reactors_list first and only calls
list_del() when the reactor is found. test_unregister_nonexistent
documents and guards this behaviour.
--
Best wishes,
Wen
^ permalink raw reply
* Re: [PATCH 2/3] rv/reactors: add KUnit tests for reactor_printk
From: Wen Yang @ 2026-07-09 16:53 UTC (permalink / raw)
To: Gabriele Monaco; +Cc: Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <97dc3ac923f1910a6103665fa5d69aca28430b6a.camel@redhat.com>
On 7/6/26 22:41, Gabriele Monaco wrote:
>
>
> On Tue, 2026-06-16 at 00:44 +0800, wen.yang@linux.dev wrote:
>> From: Wen Yang <wen.yang@linux.dev>
>>
>> Add KUnit tests for the printk reactor covering:
>> - Reactor registration and unregistration lifecycle
>> - React callback invocation via rv_react()
>> - Double registration rejection
>> - Multiple register/unregister cycles
>>
>> The mock callback calls vprintk_deferred() — the same path as the real
>> reactor — then busy-waits to simulate I/O back-pressure, exercising the
>> LD_WAIT_FREE constraint of rv_react() under load.
>>
>> Signed-off-by: Wen Yang <wen.yang@linux.dev>
>> ---
>> kernel/trace/rv/Kconfig | 10 ++
>> kernel/trace/rv/Makefile | 1 +
>> kernel/trace/rv/reactor_printk_kunit.c | 123 +++++++++++++++++++++++++
>> 3 files changed, 134 insertions(+)
>> create mode 100644 kernel/trace/rv/reactor_printk_kunit.c
>>
>> diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
>> index 3884b14df375..ff47895c897f 100644
>> --- a/kernel/trace/rv/Kconfig
>> +++ b/kernel/trace/rv/Kconfig
>> @@ -104,6 +104,16 @@ config RV_REACT_PRINTK
>> Enables the printk reactor. The printk reactor emits a printk()
>> message if an exception is found.
>>
>> +config RV_REACT_PRINTK_KUNIT
>> + bool "KUnit tests for reactor_printk" if !KUNIT_ALL_TESTS
>> + depends on RV_REACT_PRINTK && KUNIT
>> + default KUNIT_ALL_TESTS
>> + help
>> + This builds KUnit tests for the printk reactor. These are only
>> + for development and testing, not for regular kernel use cases.
>> +
>> + If unsure, say N.
>> +
>> config RV_REACT_PANIC
>> bool "Panic reactor"
>> depends on RV_REACTORS
>> diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
>> index 94498da35b37..ef0a2dcb927c 100644
>> --- a/kernel/trace/rv/Makefile
>> +++ b/kernel/trace/rv/Makefile
>> @@ -23,4 +23,5 @@ obj-$(CONFIG_RV_MON_NOMISS) += monitors/nomiss/nomiss.o
>> # Add new monitors here
>> obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
>> obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o
>> +obj-$(CONFIG_RV_REACT_PRINTK_KUNIT) += reactor_printk_kunit.o
>> obj-$(CONFIG_RV_REACT_PANIC) += reactor_panic.o
>> diff --git a/kernel/trace/rv/reactor_printk_kunit.c
>> b/kernel/trace/rv/reactor_printk_kunit.c
>> new file mode 100644
>> index 000000000000..933aa5602226
>> --- /dev/null
>> +++ b/kernel/trace/rv/reactor_printk_kunit.c
>> @@ -0,0 +1,123 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * KUnit tests for reactor_printk
>> + *
>> + */
>> +
>> +#include <kunit/test.h>
>> +#include <linux/rv.h>
>> +#include <linux/printk.h>
>> +#include <linux/sched/clock.h>
>> +#include <linux/processor.h>
>> +
>> +/*
>> + * Simulated execution time for mock_printk_react (sched_clock units,
>> + * nanoseconds). Models the time a real printk reactor callback may consume
>> + * under I/O pressure, exercising the LD_WAIT_FREE constraint of rv_react().
>> + */
>> +#define MOCK_REACT_DURATION_NS 5000000ULL
>> +
>> +/*
>> + * Mock react callback mirroring rv_printk_reaction().
>> + *
>> + * Calls vprintk_deferred() — the same path as the real reactor — then holds
>> + * the CPU for MOCK_REACT_DURATION_NS via a sched_clock() timed busy-loop,
>> + * simulating a callback that is slow due to I/O back-pressure.
>> + * sched_clock() is notrace and lock-free; no sleep or lock acquisition is
>> + * performed, satisfying the LD_WAIT_FREE constraint of rv_react().
>> + */
>> +__printf(1, 0) static void mock_printk_react(const char *msg, va_list args)
>> +{
>> + u64 start = sched_clock();
>> +
>
> I'm fine testing something that looks like the printk reactor rather than the
> real thing, but here sched_clock() is playing with preemption and could trigger
> one, this isn't accurate.
>
> I'm not sure all implementations are free from this problem, but why don't you
> just use mdelay() here?
>
Thanks,
v2 uses mdelay(5) for the busy-wait. The goal is simply to hold the CPU
inside rv_react()'s lockdep context; so mdelay() is a pure calibrated
spin with no scheduler interaction, making the test's causal chain
clearer than sched_clock().
The remaining comments are also very important, and we will address them
in v2.
--
Best wishes,
Wen
^ permalink raw reply
* Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test
From: Wen Yang @ 2026-07-09 17:08 UTC (permalink / raw)
To: Gabriele Monaco, linux-trace-kernel, linux-kernel, Steven Rostedt,
Shuah Khan, linux-kselftest
Cc: Nam Cao, Thomas Weissschuh, Tomas Glozar, John Kacur
In-Reply-To: <20260625121440.116317-17-gmonaco@redhat.com>
On 6/25/26 20:14, Gabriele Monaco wrote:
> The wwnr_printk test expects no reactions in some situations, after
> fixing the bash assertion, the test is failing because expecting no
> reaction after a previous step had reactions is flaky without making
> sure all buffers are flushed.
>
> Simplify the test and run the steps expecting no reaction before the one
> expecting reactions. Also simplify the load function to stop loads as
> soon as a reaction occurs, this limits the number of lines to flush and
> makes tests overall more stable.
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
> .../verification/test.d/rv_wwnr_printk.tc | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc b/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
> index 96de95edb5..a23d22f6ec 100644
> --- a/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
> +++ b/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
> @@ -4,27 +4,30 @@
> # requires: available_reactors wwnr:monitor printk:reactor stress-ng:program
>
> load() { # returns true if there was a reaction
> - local lines_before num
> + local lines_before num load_pid ret
> num=$((($(nproc) + 1) / 2))
> lines_before=$(dmesg | wc -l)
> - stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q
> - dmesg | tail -n $((lines_before + 1)) | grep -q "rv: monitor wwnr does not allow event"
> + stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q &
> + load_pid=$!
> + timeout 5 dmesg -w | tail -n +$((lines_before + 1)) | grep -m 1 -q "rv: monitor wwnr does not allow event"
Minor nit: could we add a small delay (e.g., sleep 0.1) before dmesg?
Reviewed-by: Wen Yang <wen.yang@linux.dev>
> + ret=$?
> + kill "$load_pid"
> + wait "$load_pid"
> + return $ret
> }
>
> echo 1 > monitors/wwnr/enable
> echo printk > monitors/wwnr/reactors
>
> -load
> -
> echo 0 > monitoring_on
> ! load || false
> echo 1 > monitoring_on
>
> -load
> -
> echo 0 > reacting_on
> ! load || false
> echo 1 > reacting_on
>
> +load
> +
> echo nop > monitors/wwnr/reactors
> echo 0 > monitors/wwnr/enable
^ permalink raw reply
* Re: [PATCH v3 15/17] selftests/verification: Fix wrong errexit assumption
From: Wen Yang @ 2026-07-09 17:10 UTC (permalink / raw)
To: Gabriele Monaco, linux-trace-kernel, linux-kernel, Steven Rostedt,
Shuah Khan, linux-kselftest
Cc: Nam Cao, Thomas Weissschuh, Tomas Glozar, John Kacur
In-Reply-To: <20260625121440.116317-16-gmonaco@redhat.com>
On 6/25/26 20:14, Gabriele Monaco wrote:
> RV selftest rely on bash errexit (set -e) to terminate with error, when
> a step is expected to return false, the following syntax is used:
>
> ! cmd
>
> This however prevents the test from exiting when cmd is false (desired)
> but doesn't exit if cmd is true, since commands prefixed with ! are
> explicitly excluded from errexit.
>
> Use the syntax
>
> ! cmd || false
>
> Which ends up checking the exit value of ! cmd and supplies a false
> command for errexit to evaluate.
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Wen Yang <wen.yang@linux.dev>
--
Best wishes,
Wen
^ permalink raw reply
* Re: [PATCH v3 06/17] verification/rvgen: Add selftests
From: Wen Yang @ 2026-07-09 17:13 UTC (permalink / raw)
To: Gabriele Monaco, linux-trace-kernel, linux-kernel, Steven Rostedt
Cc: Nam Cao, Thomas Weissschuh, Tomas Glozar, John Kacur
In-Reply-To: <20260625121440.116317-7-gmonaco@redhat.com>
On 6/25/26 20:14, Gabriele Monaco wrote:
> The rvgen code generator needs validation to ensure it produces correct
> monitor implementations from input specifications.
>
> Add selftests with golden reference outputs covering all monitor classes
> (DA, HA, LTL) and types (global, per_cpu, per_task, per_obj), including
> optional features like descriptions and parent monitors. Container
> generation and error handling (missing files, invalid specifications,
> missing arguments) are also validated against expected output.
>
> Acked-by: Nam Cao <namcao@linutronix.de>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Wen Yang <wen.yang@linux.dev>
--
Best wishes,
Wen
^ permalink raw reply
* Re: [PATCH 1/3] rv/reactors: fix lockdep "Invalid wait context" in rv_react()
From: Nam Cao @ 2026-07-09 18:07 UTC (permalink / raw)
To: Wen Yang, Thomas Weißschuh, Gabriele Monaco
Cc: linux-trace-kernel, linux-kernel
In-Reply-To: <397cea12-e0ba-4cf5-a411-26f44bc17d01@linux.dev>
Wen Yang <wen.yang@linux.dev> writes:
> How about a context-sensitive approach, eg:
>
> NMI/hardirq (interrupts masked, scheduler cannot run):
> Keep LD_WAIT_FREE. The false positive cannot arise in this path,
> and the original constraint against raw spinlocks is preserved where
> it is meaningful.
>
> task/softirq/PREEMPT_RT irq thread (preemptible):
> Raise to LD_WAIT_SPIN. Raw spinlocks become permitted — as Gabriele
> note, this is a necessary consequence of any fix in this path.
I don't get the point. Unless the lock's type is also context-sensitive,
the reactors still cannot use raw spin locks.
Nam
^ permalink raw reply
* Re: [PATCH v3 04/11] arm64/mm: Add set_memory_device() and set_memory_normal()
From: Thierry Reding @ 2026-07-09 19:58 UTC (permalink / raw)
To: Will Deacon
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Hunter,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Sowjanya Komatineni, Luca Ceresoli,
Mikko Perttunen, Yury Norov, Rasmus Villemoes, Russell King,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Marek Szyprowski, Robin Murphy, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Catalin Marinas, Thierry Reding, devicetree, linux-tegra,
linux-kernel, dri-devel, linux-media, linux-arm-kernel,
linux-s390, linux-mm, iommu, linaro-mm-sig, linux-trace-kernel,
Thierry Reding, Chun Ng
In-Reply-To: <akzikTrmhMsvkNVY@willie-the-truck>
[-- Attachment #1: Type: text/plain, Size: 5485 bytes --]
On Tue, Jul 07, 2026 at 12:27:13PM +0100, Will Deacon wrote:
> On Mon, Jul 06, 2026 at 03:49:24PM +0200, Thierry Reding wrote:
> > On Fri, Jul 03, 2026 at 06:13:31PM +0100, Will Deacon wrote:
> > > On Thu, Jul 02, 2026 at 06:41:23PM +0200, Thierry Reding wrote:
> > > > On Thu, Jul 02, 2026 at 03:46:44PM +0200, Thierry Reding wrote:
> > > > > On Thu, Jul 02, 2026 at 10:18:47AM +0100, Will Deacon wrote:
> > > > > > On Wed, Jul 01, 2026 at 06:08:15PM +0200, Thierry Reding wrote:
> > > > > > > From: Chun Ng <chunn@nvidia.com>
> > > > > > >
> > > > > > > Add helpers to swap PROT_NORMAL and PROT_DEVICE_nGnRnE protection bits
> > > > > > > on a kernel-linear-map range.
> > > > > >
> > > > > > That sounds like a really terrible idea. Why is this necessary and how
> > > > > > does it interact with things like load_unaligned_zeropad()?
> > > > >
> > > > > This is necessary because once the memory controller has walled off the
> > > > > new memory region the CPU must not access it under any circumstances or
> > > > > it'll cause the CPU to lock up (I think technically it'll hit an SError
> > > > > but in practice that just means it'll freeze, as far as I can tell).
> > > > >
> > > > > Probably doesn't interact well at all with load_unaligned_zeropad().
> > > > >
> > > > > > I think you should unmap the memory from the linear map and memremap()
> > > > > > it instead.
> > > > >
> > > > > Given that the memory can never be accessed by the CPU after the memory
> > > > > controller locks it down, I don't think we'll even need memremap(). The
> > > > > only thing we really need is the sg_table we hand out via the DMA BUFs
> > > > > so that they can be used by device drivers to program their DMA engines
> > > > > internally.
> > > > >
> > > > > Looking through some of the architecture code around this, shouldn't we
> > > > > simply be using set_memory_encrypted() and set_memory_decrypted() for
> > > > > this? While they might've been created for slightly other use-cases,
> > > > > they seem to be doing exactly what we want (i.e. remove the page range
> > > > > from the linear mapping and flushing it, or restoring the valid bit and
> > > > > standard permissions, respectively).
> > > >
> > > > Ah... I guess we can't do it because we're not in a realm world and so
> > > > the early checks in __set_memory_enc_dec() would return early and turn
> > > > it into a no-op.
> > > >
> > > > How about if I extract a common helper and provide set_memory_p() and
> > > > set_memory_np() in terms of those. Those are available on x86 and
> > > > PowerPC as well, so fairly standard. I suppose at that point we're
> > > > closer to set_memory_valid().
> > >
> > > Why not just call set_direct_map_invalid_noflush() +
> > > flush_tlb_kernel_range() for each page? We already have APIs for this.
> >
> > Having a "standard" helper with a fixed and documented purposed seemed
> > like a preferable approach for this particular case. We also may want to
> > make the driver that uses this buildable as a module, in which case we'd
> > need to export these rather low-level APIs. And then there's also the
> > fact that we typically call this on a rather large region of memory
> > (usually something like 512 MiB), so doing it page-by-page is rather
> > suboptimal.
> >
> > > The big challenge I see with any linear map manipulation, however, is
> > > that it will rely on can_set_direct_map() which likely means you need to
> > > give up some performance and/or security to make this work. Does memory
> > > become inaccesible dynamically at runtime? If not, the best bet would
> > > be to describe it as a carveout in the DT and mark it as "no-map" so
> > > we avoid mapping it in the first place.
> >
> > VPR exists in two modes: static and resizable. For static VPR we do
> > exactly that: describe it as carveout in DT with no-map and deal with it
> > accordingly in the driver. Resizable VPR is for device that have small
> > amounts of RAM. Content-protected video playback will in the worst case
> > consume around 1.8 GiB of RAM, so we want to be able to reuse for other
> > purposes when VPR is unused on those devices. In that case, the memory
> > is also described as a reserved-memory region in DT, but it is marked as
> > reusable so that it can be managed by CMA.
> >
> > The resize operation is fairly slow to begin with because we need to
> > stall the GPU and put it into reset before the operation, then take it
> > out of reset and resume it afterwards.
> >
> > What kind of performance impact do you expect?
>
> You'll need to measure it, but we've seen reports of double-digit
> percentage regressions in performance and power. As I said, the problem
> is that you need to split the linear map to 4k page at runtime to unmap
> the dynamic carveout, but that isn't something that can be done on most
> CPUs. Therefore you end up having to use page-granular mappings for the
> entire thing, similarly to how 'rodata_full' drives can_set_direct_map()
> and the perf/power hit affects everything.
The VPR has fairly large alignment restrictions (1 MiB) and we do unmap
in fairly large chunks (512 MiB currently, but we can change that if it
is helpful) because we really want to avoid resizing operations, so the
tradeoff is between frequency of resize vs. potential memory wasted.
Does that change anything with regards to performance?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
From: Steven Rostedt @ 2026-07-09 23:15 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team,
linux-kernel, Sashiko
In-Reply-To: <20260709160017.1729517-2-vdonnefort@google.com>
On Thu, 9 Jul 2026 17:00:15 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:
> If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus
> is not yet incremented for the current CPU. As a consequence, on error,
> half-allocated rb_desc will not be freed in trace_remote_free_buffer().
>
> Increment desc->nr_cpus as soon as the first allocation for the current
> CPU has succeeded.
>
> Fixes: 96e43537af54 ("tracing: Introduce trace remotes")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
This patch makes Sashiko find other possible issues with the code :-p
https://sashiko.dev/#/patchset/20260709160017.1729517-2-vdonnefort%40google.com
-- Steve
^ permalink raw reply
* Re: [BUG] tracing: Too many tries to read user space
From: Masami Hiramatsu @ 2026-07-10 3:22 UTC (permalink / raw)
To: Jeongho Choi
Cc: linux-trace-kernel, linux-kernel, rostedt, ji2yoon.jo, minki.jang,
hajun.sung
In-Reply-To: <20260708123753.GB1386@KORCO121415.samsungds.net>
On Wed, 8 Jul 2026 21:37:53 +0900
Jeongho Choi <jh1012.choi@samsung.com> wrote:
> Hello,
>
> We are seeing a reproducible kernel panic related to the tracing code
> when it fails to read user-space memory.
>
> The issue was originally reported through the Android/Google Issue
> Tracker, and we were advised to report it to the upstream trace mailing
> list because the affected code is upstream.
>
> Environment:
>
> Architecture: arm64
> Kernel: Linux 6.18.21
> Base: Android Common Kernel (android17-6.18)
> Affected area: kernel/trace/
>
> The relevant error/panic log is:
> [48916.569148] [9: lmkd: 536] Error: Too many tries to read
> user space
> [48916.569156] [9: lmkd: 536] WARNING: CPU: 9 PID: 536 at
> kernel/trace/trace.c:7374 trace_user_fault_read+0x334/0x360
> [48916.569443] [9: lmkd: 536] CPU: 9 UID: 1069 PID: 536 Comm:
> lmkd Tainted: G OE 6.18.21-android17-5-ga1a8e8cab9ec-4k
> #1 PREEMPT 25372cd4750dcac3c0fe86b57d47c665f97a6046
>
> [48916.569450] [9: lmkd: 536] pstate: 63402005 (nZCv daif
> +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
> [48916.569452] [9: lmkd: 536] pc :
> trace_user_fault_read+0x334/0x360
> [48916.569454] [9: lmkd: 536] lr :
> trace_user_fault_read+0x330/0x360
> [48916.569456] [9: lmkd: 536] sp : ffffffc096993cc0
> [48916.569457] [9: lmkd: 536] x29: ffffffc096993cd0 x28:
> 0000000000000065 x27: ffffff8817e45200
> [48916.569461] [9: lmkd: 536] x26: 0000000000000000 x25:
> 000000011616c082 x24: 0000000000000000
> [48916.569463] [9: lmkd: 536] x23: 0000000000000009 x22:
> 000000000000002b x21: ffffff880597f740
> [48916.569466] [9: lmkd: 536] x20: 0000007fc42dd940 x19:
> ffffff8817e45770 x18: ffffffd5947ce240
> [48916.569468] [9: lmkd: 536] x17: 2073656972742079 x16:
> 6e616d206f6f5420 x15: 3a726f727245205d
> [48916.569471] [9: lmkd: 536] x14: 36333520203a646b x13:
> 6563617073207265 x12: 0000000000000001
> [48916.569473] [9: lmkd: 536] x11: 6f74207365697274 x10:
> 0000000000000001 x9 : 4bd9ad4516d75100
> [48916.569476] [9: lmkd: 536] x8 : 4bd9ad4516d75100 x7 :
> 205d383431393635 x6 : 2e36313938345b0a
> [48916.569479] [9: lmkd: 536] x5 : ffffffc080fa5998 x4 :
> ffffffd591707202 x3 : 0001360a00000000
> [48916.569481] [9: lmkd: 536] x2 : ffffffc096993af4 x1 :
> 00000000000000c0 x0 : 0000000000000028
> [48916.569484] [9: lmkd: 536] Call trace:
> [48916.569486] [9: lmkd: 536]
> trace_user_fault_read+0x334/0x360 (P)
> [48916.569488] [9: lmkd: 536] tracing_mark_write+0x84/0x174
> [48916.569491] [9: lmkd: 536] __arm64_sys_write+0x2a0/0x5c0
> [48916.569494] [9: lmkd: 536] invoke_syscall+0x58/0xe4
> [48916.569498] [9: lmkd: 536] do_el0_svc+0x48/0xdc
> [48916.569500] [9: lmkd: 536] el0_svc+0x3c/0x98
> [48916.569503] [9: lmkd: 536]
> el0t_64_sync_handler+0x20/0x130
> [48916.569505] [9: lmkd: 536] el0t_64_sync+0x1c4/0x1c8
> [48916.569508] [9: lmkd: 536] Kernel panic - not syncing:
> kernel: panic_on_warn set ...
>
>
> The code at the WARN location mentioned in the log above is as follows.
>
> 7374 if (WARN_ONCE(trys++ > 100, "Error: Too many
> tries to read user space"))
> 7375 return NULL;
>
>
> Our current analysis is as follows:
>
> In the Gmail process, during a low memory situation, LMKD writes strings
> to /sys/kernel/tracing/trace_marker for systrace recording. At the same
> time, it broadcasts a sigkill due to low memory, which is causing the
> LMKD trace marker operation to stall.
Hm, in my view, this warning indicates that the circuit breaker has
triggered correctly, so that is not a bug. Under the heavy memory
pressure and low-memory situation, the page can be reclaimed soon
after it is copied.
However, this seems a bit strange that we only checks the CPU-wide context
switching in the loop. Instead, can we introduce a per-cpu sequence counter
to per-cpu buffer, and check it?
Could you try this ?
From f76d8e4400a5961725d17899f4290c9334987e2b Mon Sep 17 00:00:00 2001
From: Masami Hiramatsu <mhiramat@kernel.org>
Date: Fri, 10 Jul 2026 12:11:00 +0900
Subject: [PATCH] tracing: Use per-CPU sequence counter in
trace_user_fault_read
trace_user_fault_read() copies trace data from user space to the
per-CPU trace buffer. When preemption is enabled during the copy, it
checks if any context switches occurred on the current CPU via
nr_context_switches_cpu() to detect whether the buffer may have been
corrupted by another trace writer.
However, under heavy memory pressure, copying from user space can trigger
page faults (e.g., for swapped-out BSS or anonymous pages) that block and
cause a context switch. Because nr_context_switches_cpu() detects any
context switch (even unrelated ones), it mistakenly assumes the buffer was
corrupted. This leads to repeated retries (up to the 100-try limit), which
causes a WARN_ONCE backtrace and returns -EFAULT to user space, even if
no other task ever accessed the trace buffer.
To mitigate this issue, replace the CPU-wide context switch check with
a dedicated per-CPU sequence counter in struct trace_user_buf.
Since only other tasks invoking trace_user_fault_read() on the same CPU
will increment this counter, unrelated context switches (including those
from page fault sleep) will no longer trigger retries.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 1bc27c0ad029..46cec63f5798 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5989,6 +5989,7 @@ static ssize_t write_marker_to_buffer(struct trace_array *tr, const char *buf,
struct trace_user_buf {
char *buf;
+ unsigned int sequence;
};
static DEFINE_MUTEX(trace_user_buffer_mutex);
@@ -6031,7 +6032,10 @@ static int user_fault_buffer_enable(struct trace_user_buf_info *tinfo, size_t si
/* Clear each buffer in case of error */
for_each_possible_cpu(cpu) {
- per_cpu_ptr(tinfo->tbuf, cpu)->buf = NULL;
+ struct trace_user_buf *tbuf = per_cpu_ptr(tinfo->tbuf, cpu);
+
+ tbuf->buf = NULL;
+ tbuf->sequence = 0;
}
for_each_possible_cpu(cpu) {
@@ -6196,8 +6200,9 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
trace_user_buf_copy copy_func, void *data)
{
int cpu = smp_processor_id();
- char *buffer = per_cpu_ptr(tinfo->tbuf, cpu)->buf;
- unsigned int cnt;
+ struct trace_user_buf *tbuf = per_cpu_ptr(tinfo->tbuf, cpu);
+ char *buffer = tbuf->buf;
+ unsigned int seq;
int trys = 0;
int ret;
@@ -6211,10 +6216,10 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
return NULL;
/*
- * This acts similar to a seqcount. The per CPU context switches are
+ * This acts similar to a seqcount. The per CPU sequence counters are
* recorded, migration is disabled and preemption is enabled. The
* read of the user space memory is copied into the per CPU buffer.
- * Preemption is disabled again, and if the per CPU context switches count
+ * Preemption is disabled again, and if the per CPU sequence count
* is still the same, it means the buffer has not been corrupted.
* If the count is different, it is assumed the buffer is corrupted
* and reading must be tried again.
@@ -6235,7 +6240,8 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
preempt_enable_notrace();
preempt_disable_notrace();
cpu = smp_processor_id();
- buffer = per_cpu_ptr(tinfo->tbuf, cpu)->buf;
+ tbuf = per_cpu_ptr(tinfo->tbuf, cpu);
+ buffer = tbuf->buf;
}
/*
@@ -6250,8 +6256,9 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
if (WARN_ONCE(trys++ > 100, "Error: Too many tries to read user space"))
return NULL;
- /* Read the current CPU context switch counter */
- cnt = nr_context_switches_cpu(cpu);
+ /* Increment the per-CPU buffer sequence counter */
+ tbuf->sequence++;
+ seq = tbuf->sequence;
/*
* Preemption is going to be enabled, but this task must
@@ -6282,12 +6289,12 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
return NULL;
/*
- * Preemption is disabled again, now check the per CPU context
- * switch counter. If it doesn't match, then another user space
+ * Preemption is disabled again, now check the per CPU sequence
+ * counter. If it doesn't match, then another user space
* process may have schedule in and corrupted our buffer. In that
* case the copying must be retried.
*/
- } while (nr_context_switches_cpu(cpu) != cnt);
+ } while (tbuf->sequence != seq);
return buffer;
}
--
2.43.0
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply related
* Re: [RFC v2 1/3] arm64: kprobes: Only handle faults originating from XOL slot
From: Masami Hiramatsu @ 2026-07-10 4:10 UTC (permalink / raw)
To: Pu Hu
Cc: ada.coupriediaz@arm.com, catalin.marinas@arm.com,
davem@davemloft.net, Hongyan Xia, Jiazi Li,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
naveen@kernel.org, will@kernel.org, yang@os.amperecomputing.com
In-Reply-To: <20260709142215.226872-2-hupu@transsion.com>
On Thu, 9 Jul 2026 14:22:23 +0000
Pu Hu <hupu@transsion.com> wrote:
> From: Pu Hu <hupu@transsion.com>
>
> kprobe_fault_handler() currently treats any page fault taken while in
> KPROBE_HIT_SS or KPROBE_REENTER state as a kprobe single-step fault. This
> assumption does not hold: perf or tracing code may run from the debug
> exception path during the single-step window and take its own page fault.
>
> When the fault is handled as a kprobe fault, the PC is rewritten to the
> probe address, corrupting the exception recovery context for the real
> fault. A typical reproducer is running perf with preemptirq tracepoints
> and dwarf callchains while a kprobe is installed on a frequently
> executed function.
>
> Fix this in two layers:
>
> 1. At function entry, bail out immediately for simulated kprobes
> (ainsn.xol_insn == NULL), since they have no XOL slot and any fault
> taken during their execution cannot be a single-step fault.
>
> 2. For kprobes with an XOL slot, only handle the fault when the
> faulting PC matches the XOL instruction address. Faults from any
> other PC are left to the normal page fault handler.
>
> This follows the same principle as the x86 fix in commit 6381c24cd6d5
> ("kprobes/x86: Fix page-fault handling logic").
>
> Signed-off-by: Pu Hu <hupu@transsion.com>
> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
This looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks,
> ---
> arch/arm64/kernel/probes/kprobes.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 43a0361a8bf0..798e4b091d1a 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -282,9 +282,31 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
> struct kprobe *cur = kprobe_running();
> struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
>
> + /*
> + * Simulated kprobes execute in the debug trap context and have no
> + * XOL slot. Any page fault taken while a simulated kprobe is in
> + * progress cannot have been caused by kprobe single-stepping and
> + * must be left alone for the normal page fault handler, including
> + * fixup_exception.
> + */
> + if (cur && !cur->ainsn.xol_insn)
> + return 0;
> +
> switch (kcb->kprobe_status) {
> case KPROBE_HIT_SS:
> case KPROBE_REENTER:
> + /*
> + * A page fault taken while in KPROBE_HIT_SS or
> + * KPROBE_REENTER state is only attributable to kprobe
> + * single-stepping if the faulting PC points to the
> + * current kprobe's XOL instruction. If the fault occurred
> + * elsewhere (e.g. in perf or tracing code invoked from the
> + * debug exception path), leave it for the normal page fault
> + * handler to process.
> + */
> + if (instruction_pointer(regs) != (unsigned long)cur->ainsn.xol_insn)
> + break;
> +
> /*
> * We are here because the instruction being single
> * stepped caused a page fault. We reset the current
> --
> 2.43.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [RFC v2 2/3] arm64: kprobes: Allow reentering kprobes while single-stepping
From: Masami Hiramatsu @ 2026-07-10 5:09 UTC (permalink / raw)
To: Pu Hu
Cc: ada.coupriediaz@arm.com, catalin.marinas@arm.com,
davem@davemloft.net, Hongyan Xia, Jiazi Li,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
naveen@kernel.org, will@kernel.org, yang@os.amperecomputing.com
In-Reply-To: <20260709142215.226872-3-hupu@transsion.com>
On Thu, 9 Jul 2026 14:22:25 +0000
Pu Hu <hupu@transsion.com> wrote:
> From: Pu Hu <hupu@transsion.com>
>
> A kprobe can be hit while another kprobe is in KPROBE_HIT_SS state. This
> can happen when tracing or perf code runs from the debug exception path
> while the first kprobe is preparing or executing its out-of-line
> single-step instruction.
>
> Currently arm64 treats a kprobe hit in KPROBE_HIT_SS as unrecoverable,
> the same as a hit in KPROBE_REENTER. This is too strict. A hit in
> KPROBE_HIT_SS is still a one-level reentry and can be handled by saving
> the current kprobe state and setting up single-step for the new probe,
> just like reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
>
> The truly unrecoverable case is hitting another kprobe while already in
> KPROBE_REENTER, because the reentry save area has already been consumed.
>
> Move KPROBE_HIT_SS to the recoverable reentry cases and leave
> KPROBE_REENTER as the unrecoverable nested reentry case.
>
> This mirrors the x86 fix in commit 6a5022a56ac3
> ("kprobes/x86: Allow to handle reentered kprobe on single-stepping").
>
Hi, as Sashiko commented, we have to save the saved_irqflag to
prev_kprobbe.
https://sashiko.dev/#/patchset/20260709142215.226872-1-hupu%40transsion.com?part=2
Thank you,
> Signed-off-by: Pu Hu <hupu@transsion.com>
> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
> ---
> arch/arm64/kernel/probes/kprobes.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 798e4b091d1a..2ca5916eca2f 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -240,10 +240,16 @@ static int __kprobes reenter_kprobe(struct kprobe *p,
> switch (kcb->kprobe_status) {
> case KPROBE_HIT_SSDONE:
> case KPROBE_HIT_ACTIVE:
> + case KPROBE_HIT_SS:
> + /*
> + * A probe can be hit while another kprobe is preparing or
> + * executing its XOL single-step instruction. This is still a
> + * recoverable one-level reentry, so handle it in the same way as
> + * reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
> + */
> kprobes_inc_nmissed_count(p);
> setup_singlestep(p, regs, kcb, 1);
> break;
> - case KPROBE_HIT_SS:
> case KPROBE_REENTER:
> pr_warn("Failed to recover from reentered kprobes.\n");
> dump_kprobe(p);
> --
> 2.43.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox