* Re: [PATCH v2 10/33] mm/rmap: rename anon_vma_interval_tree_*() params and use pgoff_t
From: Vlastimil Babka (SUSE) @ 2026-07-13 18:05 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-10-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> Rename parameters used by anon_vma_interval_tree_*() functions: 'node' to
> 'avc', 'start/first' to 'pgoff_start', and 'last' to 'pgoff_last' to make
> clear what is being passed.
>
> Also, express page offsets in terms of pgoff_t to be consistent.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 11/33] mm/rmap: rename anon_vma_interval_tree_*() to anon_rmap_tree_*()
From: Vlastimil Babka (SUSE) @ 2026-07-13 18:06 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-11-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> To be consistent with the newly renamed mapping_rmap_tree_*(), rename the
> anon_vma_interval_tree_*() helpers to anon_rmap_tree_*().
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 12/33] MAINTAINERS: Move mm/interval_tree.c to rmap section
From: Vlastimil Babka (SUSE) @ 2026-07-13 18:07 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-12-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> This file implements code for the interval trees used by the file and anon
> rmap implementation, so belongs in the rmap section.
>
> Acked-by: Pedro Falcato <pfalcato@suse.de>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
> MAINTAINERS | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c3e535ca4bef..ef168b583325 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17208,6 +17208,7 @@ R: Lance Yang <lance.yang@linux.dev>
> L: linux-mm@kvack.org
> S: Maintained
> F: include/linux/rmap.h
> +F: mm/interval_tree.c
> F: mm/page_vma_mapped.c
> F: mm/rmap.c
> F: tools/testing/selftests/mm/rmap.c
> @@ -17313,7 +17314,6 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> F: include/trace/events/mmap.h
> F: fs/proc/task_mmu.c
> F: fs/proc/task_nommu.c
> -F: mm/interval_tree.c
> F: mm/mincore.c
> F: mm/mlock.c
> F: mm/mmap.c
>
^ permalink raw reply
* Re: [PATCH v2 13/33] mm/vma: introduce and use vmg_pages(), vmg_[start, end]_pgoff()
From: Vlastimil Babka (SUSE) @ 2026-07-13 18:12 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-13-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> In the VMA logic we often need to determine the number of pages in the
> specified merge range, as well as the start and end page offsets of that
> range.
>
> Introduce and use helpers for these purposes.
>
> No functional change intended.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Uh, the PHYS_PFN() usage here was quite an abuse. Good riddance.
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
> mm/vma.c | 11 ++++-------
> mm/vma.h | 17 +++++++++++++++++
> 2 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/mm/vma.c b/mm/vma.c
> index 35ba3475128f..995de8198fbb 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -197,11 +197,9 @@ static void init_multi_vma_prep(struct vma_prepare *vp,
> */
> static bool can_vma_merge_before(struct vma_merge_struct *vmg)
> {
> - pgoff_t pglen = PHYS_PFN(vmg->end - vmg->start);
> -
> if (is_mergeable_vma(vmg, /* merge_next = */ true) &&
> is_mergeable_anon_vma(vmg, /* merge_next = */ true)) {
> - if (vmg->next->vm_pgoff == vmg->pgoff + pglen)
> + if (vmg_end_pgoff(vmg) == vma_start_pgoff(vmg->next))
> return true;
> }
>
> @@ -221,7 +219,7 @@ static bool can_vma_merge_after(struct vma_merge_struct *vmg)
> {
> if (is_mergeable_vma(vmg, /* merge_next = */ false) &&
> is_mergeable_anon_vma(vmg, /* merge_next = */ false)) {
> - if (vmg->prev->vm_pgoff + vma_pages(vmg->prev) == vmg->pgoff)
> + if (vma_end_pgoff(vmg->prev) == vmg_start_pgoff(vmg))
> return true;
> }
> return false;
> @@ -759,7 +757,7 @@ static int commit_merge(struct vma_merge_struct *vmg)
> */
> vma_adjust_trans_huge(vma, vmg->start, vmg->end,
> vmg->__adjust_middle_start ? vmg->middle : NULL);
> - vma_set_range(vma, vmg->start, vmg->end, vmg->pgoff);
> + vma_set_range(vma, vmg->start, vmg->end, vmg_start_pgoff(vmg));
> vmg_adjust_set_range(vmg);
> vma_iter_store_overwrite(vmg->vmi, vmg->target);
>
> @@ -962,8 +960,7 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
> * middle next
> * shrink/delete extend
> */
> -
> - pgoff_t pglen = PHYS_PFN(vmg->end - vmg->start);
> + const pgoff_t pglen = vmg_pages(vmg);
>
> VM_WARN_ON_VMG(!merge_right, vmg);
> /* If we are offset into a VMA, then prev must be middle. */
> diff --git a/mm/vma.h b/mm/vma.h
> index 8e4b61a7304c..527716c8739d 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -230,6 +230,23 @@ static inline bool vmg_nomem(struct vma_merge_struct *vmg)
> return vmg->state == VMA_MERGE_ERROR_NOMEM;
> }
>
> +static inline pgoff_t vmg_start_pgoff(const struct vma_merge_struct *vmg)
> +{
> + return vmg->pgoff;
> +}
> +
> +static inline pgoff_t vmg_pages(const struct vma_merge_struct *vmg)
> +{
> + const unsigned long size = vmg->end - vmg->start;
> +
> + return size >> PAGE_SHIFT;
> +}
> +
> +static inline pgoff_t vmg_end_pgoff(const struct vma_merge_struct *vmg)
> +{
> + return vmg_start_pgoff(vmg) + vmg_pages(vmg);
> +}
> +
> /* Assumes addr >= vma->vm_start. */
> static inline pgoff_t vma_pgoff_offset(struct vm_area_struct *vma,
> unsigned long addr)
>
^ permalink raw reply
* Re: [PATCH v2 14/33] mm/vma: clean up anon_vma_compatible()
From: Vlastimil Babka (SUSE) @ 2026-07-13 18:18 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-14-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> Break up the existing very large conditional, add comments and use
> vma_[start/end]_pgoff() to make clearer what we're doing here.
>
> No functional change intended.
>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
> mm/vma.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/mm/vma.c b/mm/vma.c
> index 995de8198fbb..084afdf37385 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -1967,14 +1967,25 @@ static int anon_vma_compatible(struct vm_area_struct *a, struct vm_area_struct *
> {
> vma_flags_t diff = vma_flags_diff_pair(&a->flags, &b->flags);
>
> + /* Ignore flags that mprotect() can change. */
> vma_flags_clear_mask(&diff, VMA_ACCESS_FLAGS);
> + /* Ignore flags that do not impact merging. */
> vma_flags_clear_mask(&diff, VMA_IGNORE_MERGE_FLAGS);
>
> - return a->vm_end == b->vm_start &&
> - mpol_equal(vma_policy(a), vma_policy(b)) &&
> - a->vm_file == b->vm_file &&
> - vma_flags_empty(&diff) &&
> - b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
> + /* Must be adjacent. */
> + if (a->vm_end != b->vm_start)
> + return false;
> + /* Must have matching policy. */
> + if (!mpol_equal(vma_policy(a), vma_policy(b)))
> + return false;
> + /* Must both be anon or map the same file (MAP_PRIVATE case). */
> + if (a->vm_file != b->vm_file)
> + return false;
> + /* Flags must be equivalent modulo mprotect(). */
> + if (!vma_flags_empty(&diff))
> + return false;
> + /* Page offset must align. */
> + return vma_end_pgoff(a) == vma_start_pgoff(b);
> }
>
> /*
>
^ permalink raw reply
* Re: [PATCH] tracing: Add mutex to trace_parser to fix concurrent write races
From: Steven Rostedt @ 2026-07-13 19:36 UTC (permalink / raw)
To: Tengda Wu
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel
In-Reply-To: <20260713134640.708323-1-wutengda@huaweicloud.com>
On Mon, 13 Jul 2026 13:46:40 +0000
Tengda Wu <wutengda@huaweicloud.com> wrote:
> The trace_parser structure is allocated and initialized when a trace
> file is opened, and is subsequently used in the write handler to parse
> user input. If userspace opens a trace file descriptor and shares it
> across multiple threads, concurrent write calls will race on the
> parser's internal state, specifically the idx, cont, and buffer fields,
> leading to corrupted input or undefined behavior.
>
> Fix this by embedding a mutex directly in struct trace_parser. The mutex
> is initialized in trace_parser_get_init() and destroyed in
> trace_parser_put(). All write-side users that access parser state
> (trace_get_user() followed by checking trace_parser_loaded() /
> trace_parser_cont() against the buffer) now hold the mutex across the
> full critical section, avoiding any TOCTOU gap between the parse and the
> subsequent consumption of parser->buffer.
>
> Affected write paths:
> - ftrace_graph_write / ftrace_graph_release
> - ftrace_regex_write / ftrace_regex_release
>
> Fixes: e704eff3ff51 ("ftrace: Have set_graph_function handle multiple functions in one write")
> Fixes: 689fd8b65d66 ("tracing: trace parser support for function and graph")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> ---
> kernel/trace/ftrace.c | 7 +++++++
> kernel/trace/trace.c | 2 ++
> kernel/trace/trace.h | 1 +
> 3 files changed, 10 insertions(+)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index f93e34dd2328..ef47e5659283 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5842,6 +5842,8 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
> /* iter->hash is a local copy, so we don't need regex_lock */
>
> parser = &iter->parser;
> +
> + guard(mutex)(&parser->lock);
> read = trace_get_user(parser, ubuf, cnt, ppos);
Why are the other users of trace_get_user() not a problem? If
anything, trace_get_user() should have a lockdep assert to make sure
the lock is held.
I think we need to add a lockdep assertion in all the callers that use
the parser and we need to make sure it's taken by every user.
-- Steve
^ permalink raw reply
* [PATCH v3 0/5] tracing/probes: Clean up and refactor argument parser
From: Masami Hiramatsu (Google) @ 2026-07-14 1:09 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
Hi,
This is the 3rd version of the series which refactors and cleans up
the fetcharg parser in the trace_probe. The previous version is here:
https://lore.kernel.org/all/178392766002.3229912.15711868107734330750.stgit@devnote2/
The main goals are to improve code readability, prevent kernel stack
overflow by eliminating recursive function calls, and extend the
argument string length limit to support complex BTF typecast
expressions.
In this version, I fixed to remove unused variable and add a
error log in parse_this_cpu() [5/5].
Patches:
- Patch 1: Decomposes parse_probe_vars() into smaller, dedicated helper
functions for parsing specific variable types ($retval, $stack, etc.).
- Patch 2: Decomposes parse_probe_arg() into static helper subroutines for
handling registers, memory/symbols, and immediates.
- Patch 3: Alphabetizes the ERRORS macros list in trace_probe.h to improve
readability.
- Patch 4: Extends MAX_ARGSTR_LEN from 63 to 255 for long BTF cast/dereference
arguments, and introduces MAX_COMMON_HEAD_LEN to prevent regression on match
head commands.
- Patch 5: Eliminates recursion in parse_probe_arg() using a loop and a parse
state stack integrated into traceprobe_parse_context. It also raises the
nesting limit to 8 and updates the selftest scripts.
Thanks,
---
Masami Hiramatsu (5):
tracing/probes: Refactor parse_probe_vars()
tracing/probes: Refactor parse_probe_arg()
tracing/probes: Sort ERRORS list in trace_probe.h alphabetically
tracing/probes: Extend max length of argument string
tracing/probes: Eliminate recursion in parse_probe_arg()
kernel/trace/trace_fprobe.c | 2
kernel/trace/trace_kprobe.c | 2
kernel/trace/trace_probe.c | 833 ++++++++++++--------
kernel/trace/trace_probe.h | 196 +++--
kernel/trace/trace_uprobe.c | 2
.../ftrace/test.d/dynevent/fprobe_syntax_errors.tc | 6
.../ftrace/test.d/dynevent/tprobe_syntax_errors.tc | 4
.../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 6
8 files changed, 623 insertions(+), 428 deletions(-)
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v3 1/5] tracing/probes: Refactor parse_probe_vars()
From: Masami Hiramatsu (Google) @ 2026-07-14 1:09 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
In-Reply-To: <178399136606.27810.4469135881050354189.stgit@devnote2>
From: Masami Hiramatsu <mhiramat@kernel.org>
Decompose parse_probe_vars() by extracting retval, stack, current task
struct, and function argument parsing logic into dedicated static
helper functions (parse_probe_var_retval, parse_probe_var_stack,
parse_probe_var_current, and parse_probe_var_arg). This simplifies
parse_probe_vars() and improves its readability.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v2:
- Fix double error logging by removing err code passing, but just
record errors inside subfunctions.
- Simplify parse_probe_vars().
---
kernel/trace/trace_probe.c | 237 ++++++++++++++++++++++++++------------------
1 file changed, 140 insertions(+), 97 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 18c212122344..8af97e29f096 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1319,132 +1319,175 @@ NOKPROBE_SYMBOL(store_trace_entry_data)
#define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long))
-/* Parse $vars. @orig_arg points '$', which syncs to @ctx->offset */
-static int parse_probe_vars(char *orig_arg, const struct fetch_type *t,
- struct fetch_insn **pcode,
- struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
+static int parse_probe_var_retval(char *orig_arg, char *arg,
+ struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
{
struct fetch_insn *code = *pcode;
- int err = TP_ERR_BAD_VAR;
- char *arg = orig_arg + 1;
+
+ if (!(ctx->flags & TPARG_FL_RETURN)) {
+ trace_probe_log_err(ctx->offset, RETVAL_ON_PROBE);
+ return -EINVAL;
+ }
+ if (!(ctx->flags & TPARG_FL_KERNEL) ||
+ !IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS)) {
+ code->op = FETCH_OP_RETVAL;
+ return 0;
+ }
+ return parse_btf_arg(orig_arg, pcode, end, ctx);
+}
+
+static int parse_probe_var_stack(char *arg, int len, struct fetch_insn *code,
+ struct traceprobe_parse_context *ctx)
+{
unsigned long param;
- int ret = 0;
- int len;
+ int ret;
- if (ctx->flags & TPARG_FL_TEVENT) {
- if (parse_trace_event(arg, code, ctx) < 0) {
- /* 'comm' should be checked after field parsing. */
- if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
- code->op = FETCH_OP_COMM;
- return 0;
- }
- goto inval;
- }
+ if (arg[len] == '\0') {
+ code->op = FETCH_OP_STACKP;
return 0;
}
- if (str_has_prefix(arg, "retval")) {
- if (!(ctx->flags & TPARG_FL_RETURN)) {
- err = TP_ERR_RETVAL_ON_PROBE;
- goto inval;
+ if (isdigit(arg[len])) {
+ ret = kstrtoul(arg + len, 10, ¶m);
+ if (ret) {
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return ret;
}
- if (!(ctx->flags & TPARG_FL_KERNEL) ||
- !IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS)) {
- code->op = FETCH_OP_RETVAL;
- return 0;
+
+ if ((ctx->flags & TPARG_FL_KERNEL) &&
+ param > PARAM_MAX_STACK) {
+ trace_probe_log_err(ctx->offset, BAD_STACK_NUM);
+ return -EINVAL;
}
+ code->op = FETCH_OP_STACK;
+ code->param = (unsigned int)param;
+ return 0;
+ }
+
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return -EINVAL;
+}
+
+static int parse_probe_var_current(char *orig_arg, char *arg,
+ struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ struct fetch_insn *code = *pcode;
+
+ /* $current is only supported by kernel probe. */
+ if (!(ctx->flags & TPARG_FL_KERNEL)) {
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return -EINVAL;
+ }
+ arg += strlen("current");
+ if (*arg == '-' && IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS))
return parse_btf_arg(orig_arg, pcode, end, ctx);
+
+ if (*arg != '\0') {
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return -EINVAL;
}
- len = str_has_prefix(arg, "stack");
- if (len) {
+ code->op = FETCH_OP_CURRENT;
+ return 0;
+}
- if (arg[len] == '\0') {
- code->op = FETCH_OP_STACKP;
- return 0;
- }
+#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
+static int parse_probe_var_arg(char *arg, int len, struct fetch_insn *code,
+ struct traceprobe_parse_context *ctx)
+{
+ unsigned long param;
+ int ret;
- if (isdigit(arg[len])) {
- ret = kstrtoul(arg + len, 10, ¶m);
- if (ret)
- goto inval;
+ ret = kstrtoul(arg + len, 10, ¶m);
+ if (ret) {
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return ret;
+ }
- if ((ctx->flags & TPARG_FL_KERNEL) &&
- param > PARAM_MAX_STACK) {
- err = TP_ERR_BAD_STACK_NUM;
- goto inval;
- }
- code->op = FETCH_OP_STACK;
- code->param = (unsigned int)param;
- return 0;
- }
- goto inval;
+ if (!param || param > PARAM_MAX_STACK) {
+ trace_probe_log_err(ctx->offset, BAD_ARG_NUM);
+ return -EINVAL;
}
+ param--; /* argN starts from 1, but internal arg[N] starts from 0 */
- if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
- code->op = FETCH_OP_COMM;
- return 0;
+ if (tparg_is_function_entry(ctx->flags)) {
+ code->op = FETCH_OP_ARG;
+ code->param = (unsigned int)param;
+ /*
+ * The tracepoint probe will probe a stub function, and the
+ * first parameter of the stub is a dummy and should be ignored.
+ */
+ if (ctx->flags & TPARG_FL_TPOINT)
+ code->param++;
+ } else if (tparg_is_function_return(ctx->flags)) {
+ /* function entry argument access from return probe */
+ ret = __store_entry_arg(ctx->tp, param);
+ if (ret < 0) /* This error should be an internal error */
+ return ret;
+
+ code->op = FETCH_OP_EDATA;
+ code->offset = ret;
+ } else {
+ trace_probe_log_err(ctx->offset, NOFENTRY_ARGS);
+ return -EINVAL;
}
+ return 0;
+}
+#else
+static int parse_probe_var_arg(char *arg, int len, struct fetch_insn *code,
+ struct traceprobe_parse_context *ctx)
+{
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return -EINVAL;
+}
+#endif
- /* $current returns the address of the current task_struct. */
- if (str_has_prefix(arg, "current")) {
- /* $current is only supported by kernel probe. */
- if (!(ctx->flags & TPARG_FL_KERNEL)) {
- err = TP_ERR_BAD_VAR;
- goto inval;
- }
- arg += strlen("current");
- if (*arg == '-' && IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS))
- return parse_btf_arg(orig_arg, pcode, end, ctx);
+/* Parse $vars. @orig_arg points '$', which syncs to @ctx->offset */
+static int parse_probe_vars(char *orig_arg, const struct fetch_type *t,
+ struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ struct fetch_insn *code = *pcode;
+ char *arg = orig_arg + 1;
+ int len, ret;
- if (*arg != '\0')
- goto inval;
+ if (ctx->flags & TPARG_FL_TEVENT) {
+ ret = parse_trace_event(arg, code, ctx);
+ if (!ret)
+ return 0;
+ }
- code->op = FETCH_OP_CURRENT;
+ if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
+ code->op = FETCH_OP_COMM;
return 0;
}
-#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
- len = str_has_prefix(arg, "arg");
- if (len) {
- ret = kstrtoul(arg + len, 10, ¶m);
- if (ret)
- goto inval;
+ /* eprobe only support event fields or '$comm'. */
+ if (ctx->flags & TPARG_FL_TEVENT)
+ goto inval;
- if (!param || param > PARAM_MAX_STACK) {
- err = TP_ERR_BAD_ARG_NUM;
- goto inval;
- }
- param--; /* argN starts from 1, but internal arg[N] starts from 0 */
+ if (str_has_prefix(arg, "retval"))
+ return parse_probe_var_retval(orig_arg, arg, pcode, end, ctx);
- if (tparg_is_function_entry(ctx->flags)) {
- code->op = FETCH_OP_ARG;
- code->param = (unsigned int)param;
- /*
- * The tracepoint probe will probe a stub function, and the
- * first parameter of the stub is a dummy and should be ignored.
- */
- if (ctx->flags & TPARG_FL_TPOINT)
- code->param++;
- } else if (tparg_is_function_return(ctx->flags)) {
- /* function entry argument access from return probe */
- ret = __store_entry_arg(ctx->tp, param);
- if (ret < 0) /* This error should be an internal error */
- return ret;
+ len = str_has_prefix(arg, "stack");
+ if (len)
+ return parse_probe_var_stack(arg, len, code, ctx);
- code->op = FETCH_OP_EDATA;
- code->offset = ret;
- } else {
- err = TP_ERR_NOFENTRY_ARGS;
- goto inval;
- }
- return 0;
- }
-#endif
+ /* $current returns the address of the current task_struct. */
+ if (str_has_prefix(arg, "current"))
+ return parse_probe_var_current(orig_arg, arg, pcode, end, ctx);
+
+ len = str_has_prefix(arg, "arg");
+ if (len)
+ return parse_probe_var_arg(arg, len, code, ctx);
inval:
- __trace_probe_log_err(ctx->offset, err);
+ trace_probe_log_err(ctx->offset, BAD_VAR);
return -EINVAL;
}
^ permalink raw reply related
* [PATCH v3 2/5] tracing/probes: Refactor parse_probe_arg()
From: Masami Hiramatsu (Google) @ 2026-07-14 1:09 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
In-Reply-To: <178399136606.27810.4469135881050354189.stgit@devnote2>
From: Masami Hiramatsu <mhiramat@kernel.org>
Decompose parse_probe_arg() by extracting register, memory/symbol,
dereference, immediate, and default BTF/CPU parsing handlers into
dedicated static helper functions (parse_probe_arg_register,
parse_probe_arg_mem_symbol, parse_probe_arg_deref, parse_probe_arg_imm,
and parse_probe_arg_default). This modularizes the recursive argument
parser and improves readability.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace_probe.c | 300 ++++++++++++++++++++++++++------------------
1 file changed, 174 insertions(+), 126 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 8af97e29f096..eb9f7bd13c19 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1516,158 +1516,206 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
return 0;
}
-/* Recursive argument parser */
-static int
-parse_probe_arg(char *arg, const struct fetch_type *type,
- struct fetch_insn **pcode, struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
+static int parse_probe_arg_register(char *arg, struct fetch_insn *code,
+ struct traceprobe_parse_context *ctx)
+{
+ int ret;
+
+ if (ctx->flags & (TPARG_FL_TEVENT | TPARG_FL_FPROBE)) {
+ /* eprobe and fprobe do not handle registers */
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return -EINVAL;
+ }
+ ret = regs_query_register_offset(arg + 1);
+ if (ret >= 0) {
+ code->op = FETCH_OP_REG;
+ code->param = (unsigned int)ret;
+ return 0;
+ }
+ trace_probe_log_err(ctx->offset, BAD_REG_NAME);
+ return -EINVAL;
+}
+
+static int parse_probe_arg_mem_symbol(char *arg, struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
{
struct fetch_insn *code = *pcode;
unsigned long param;
+ long offset = 0;
+ int ret;
+
+ if (isdigit(arg[1])) {
+ ret = kstrtoul(arg + 1, 0, ¶m);
+ if (ret) {
+ trace_probe_log_err(ctx->offset, BAD_MEM_ADDR);
+ return ret;
+ }
+ /* load address */
+ code->op = FETCH_OP_IMM;
+ code->immediate = param;
+ } else if (arg[1] == '+') {
+ /* Kernel probes do not support file offsets */
+ if (ctx->flags & TPARG_FL_KERNEL) {
+ trace_probe_log_err(ctx->offset, FILE_ON_KPROBE);
+ return -EINVAL;
+ }
+ ret = kstrtol(arg + 2, 0, &offset);
+ if (ret) {
+ trace_probe_log_err(ctx->offset, BAD_FILE_OFFS);
+ return ret;
+ }
+
+ code->op = FETCH_OP_FOFFS;
+ code->immediate = (unsigned long)offset;
+ offset = 0;
+ } else {
+ /* uprobes don't support symbols */
+ if (!(ctx->flags & TPARG_FL_KERNEL)) {
+ trace_probe_log_err(ctx->offset, SYM_ON_UPROBE);
+ return -EINVAL;
+ }
+ /* Preserve symbol for updating */
+ code->op = FETCH_NOP_SYMBOL;
+ code->data = kstrdup(arg + 1, GFP_KERNEL);
+ if (!code->data)
+ return -ENOMEM;
+ if (++code == end) {
+ trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ return -EINVAL;
+ }
+ code->op = FETCH_OP_IMM;
+ code->immediate = 0;
+ }
+ /* These are fetching from memory */
+ if (++code == end) {
+ trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ return -EINVAL;
+ }
+ *pcode = code;
+ code->op = FETCH_OP_DEREF;
+ code->offset = offset;
+ return 0;
+}
+
+static int parse_probe_arg_deref(char *arg, struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
int deref = FETCH_OP_DEREF;
long offset = 0;
char *tmp;
- int ret = 0;
+ int ret;
- switch (arg[0]) {
- case '$':
- ret = parse_probe_vars(arg, type, pcode, end, ctx);
- break;
+ if (arg[1] == 'u') {
+ deref = FETCH_OP_UDEREF;
+ arg[1] = arg[0];
+ arg++;
+ }
+ if (arg[0] == '+')
+ arg++; /* Skip '+', because kstrtol() rejects it. */
+ tmp = strchr(arg, '(');
+ if (!tmp) {
+ trace_probe_log_err(ctx->offset, DEREF_NEED_BRACE);
+ return -EINVAL;
+ }
+ *tmp = '\0';
+ ret = kstrtol(arg, 0, &offset);
+ if (ret) {
+ trace_probe_log_err(ctx->offset, BAD_DEREF_OFFS);
+ return ret;
+ }
+ ctx->offset += (tmp + 1 - arg) + (arg[0] != '-' ? 1 : 0);
+ arg = tmp + 1;
+ return handle_dereference(arg, pcode, end, ctx, deref, offset);
+}
- case '%': /* named register */
- if (ctx->flags & (TPARG_FL_TEVENT | TPARG_FL_FPROBE)) {
- /* eprobe and fprobe do not handle registers */
- trace_probe_log_err(ctx->offset, BAD_VAR);
- break;
+static int parse_probe_arg_imm(char *arg, struct fetch_insn *code,
+ struct traceprobe_parse_context *ctx)
+{
+ char *tmp;
+ int ret;
+
+ if (arg[1] == '"') { /* Immediate string */
+ ret = __parse_imm_string(arg + 2, &tmp, ctx->offset + 2);
+ if (ret)
+ return ret;
+ code->op = FETCH_OP_IMMSTR;
+ code->data = tmp;
+ } else {
+ ret = str_to_immediate(arg + 1, &code->immediate);
+ if (ret) {
+ trace_probe_log_err(ctx->offset + 1, BAD_IMM);
+ return ret;
}
- ret = regs_query_register_offset(arg + 1);
- if (ret >= 0) {
- code->op = FETCH_OP_REG;
- code->param = (unsigned int)ret;
- ret = 0;
- } else
- trace_probe_log_err(ctx->offset, BAD_REG_NAME);
- break;
+ code->op = FETCH_OP_IMM;
+ }
+ return 0;
+}
- case '@': /* memory, file-offset or symbol */
- if (isdigit(arg[1])) {
- ret = kstrtoul(arg + 1, 0, ¶m);
- if (ret) {
- trace_probe_log_err(ctx->offset, BAD_MEM_ADDR);
- break;
- }
- /* load address */
- code->op = FETCH_OP_IMM;
- code->immediate = param;
- } else if (arg[1] == '+') {
- /* Kernel probes do not support file offsets */
- if (ctx->flags & TPARG_FL_KERNEL) {
- trace_probe_log_err(ctx->offset, FILE_ON_KPROBE);
- return -EINVAL;
- }
- ret = kstrtol(arg + 2, 0, &offset);
- if (ret) {
- trace_probe_log_err(ctx->offset, BAD_FILE_OFFS);
- break;
- }
+static int parse_probe_arg_default(char *arg, struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ int ret;
- code->op = FETCH_OP_FOFFS;
- code->immediate = (unsigned long)offset; // imm64?
- offset = 0;
- } else {
- /* uprobes don't support symbols */
- if (!(ctx->flags & TPARG_FL_KERNEL)) {
- trace_probe_log_err(ctx->offset, SYM_ON_UPROBE);
- return -EINVAL;
- }
- /* Preserve symbol for updating */
- code->op = FETCH_NOP_SYMBOL;
- code->data = kstrdup(arg + 1, GFP_KERNEL);
- if (!code->data)
- return -ENOMEM;
- if (++code == end) {
- trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ if (str_has_prefix(arg, THIS_CPU_PTR_PREFIX) ||
+ str_has_prefix(arg, THIS_CPU_READ_PREFIX)) {
+ return parse_this_cpu(arg, pcode, end, ctx);
+ }
+
+ if (isalpha(arg[0]) || arg[0] == '_') {
+ /* BTF variable or event field */
+ if (ctx->flags & TPARG_FL_TEVENT) {
+ ret = parse_trace_event(arg, *pcode, ctx);
+ if (ret < 0) {
+ trace_probe_log_err(ctx->offset, NO_EVENT_FIELD);
return -EINVAL;
}
- code->op = FETCH_OP_IMM;
- code->immediate = 0;
+ return 0;
}
- /* These are fetching from memory */
- if (++code == end) {
- trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ if (!tparg_is_function_entry(ctx->flags) &&
+ !tparg_is_function_return(ctx->flags)) {
+ trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
return -EINVAL;
}
- *pcode = code;
- code->op = FETCH_OP_DEREF;
- code->offset = offset;
- break;
+ return parse_btf_arg(arg, pcode, end, ctx);
+ }
+ return 0;
+}
+
+/* Recursive argument parser */
+static int
+parse_probe_arg(char *arg, const struct fetch_type *type,
+ struct fetch_insn **pcode, struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ struct fetch_insn *code = *pcode;
+ int ret;
+
+ switch (arg[0]) {
+ case '$':
+ ret = parse_probe_vars(arg, type, pcode, end, ctx);
+ break;
+ case '%': /* named register */
+ ret = parse_probe_arg_register(arg, code, ctx);
+ break;
+ case '@': /* memory, file-offset or symbol */
+ ret = parse_probe_arg_mem_symbol(arg, pcode, end, ctx);
+ break;
case '+': /* deref memory */
case '-':
- if (arg[1] == 'u') {
- deref = FETCH_OP_UDEREF;
- arg[1] = arg[0];
- arg++;
- }
- if (arg[0] == '+')
- arg++; /* Skip '+', because kstrtol() rejects it. */
- tmp = strchr(arg, '(');
- if (!tmp) {
- trace_probe_log_err(ctx->offset, DEREF_NEED_BRACE);
- return -EINVAL;
- }
- *tmp = '\0';
- ret = kstrtol(arg, 0, &offset);
- if (ret) {
- trace_probe_log_err(ctx->offset, BAD_DEREF_OFFS);
- break;
- }
- ctx->offset += (tmp + 1 - arg) + (arg[0] != '-' ? 1 : 0);
- arg = tmp + 1;
- ret = handle_dereference(arg, pcode, end, ctx, deref, offset);
- if (ret < 0)
- return ret;
+ ret = parse_probe_arg_deref(arg, pcode, end, ctx);
break;
case '\\': /* Immediate value */
- if (arg[1] == '"') { /* Immediate string */
- ret = __parse_imm_string(arg + 2, &tmp, ctx->offset + 2);
- if (ret)
- break;
- code->op = FETCH_OP_IMMSTR;
- code->data = tmp;
- } else {
- ret = str_to_immediate(arg + 1, &code->immediate);
- if (ret)
- trace_probe_log_err(ctx->offset + 1, BAD_IMM);
- else
- code->op = FETCH_OP_IMM;
- }
+ ret = parse_probe_arg_imm(arg, code, ctx);
break;
case '(':
ret = handle_typecast(arg, pcode, end, ctx);
break;
default:
- if (str_has_prefix(arg, THIS_CPU_PTR_PREFIX) ||
- str_has_prefix(arg, THIS_CPU_READ_PREFIX)) {
- ret = parse_this_cpu(arg, pcode, end, ctx);
- } else if (isalpha(arg[0]) || arg[0] == '_') {
- /* BTF variable or event field*/
- if (ctx->flags & TPARG_FL_TEVENT) {
- ret = parse_trace_event(arg, *pcode, ctx);
- if (ret < 0) {
- trace_probe_log_err(ctx->offset,
- NO_EVENT_FIELD);
- return -EINVAL;
- }
- break;
- }
- if (!tparg_is_function_entry(ctx->flags) &&
- !tparg_is_function_return(ctx->flags)) {
- trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
- return -EINVAL;
- }
- ret = parse_btf_arg(arg, pcode, end, ctx);
- }
+ ret = parse_probe_arg_default(arg, pcode, end, ctx);
break;
}
if (!ret && code->op == FETCH_OP_NOP) {
^ permalink raw reply related
* [PATCH v3 3/5] tracing/probes: Sort ERRORS list in trace_probe.h alphabetically
From: Masami Hiramatsu (Google) @ 2026-07-14 1:09 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
In-Reply-To: <178399136606.27810.4469135881050354189.stgit@devnote2>
From: Masami Hiramatsu <mhiramat@kernel.org>
Sort the C-macro ERRORS list alphabetically in trace_probe.h to make
it easier to find and maintain error entries.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace_probe.h | 160 ++++++++++++++++++++++----------------------
1 file changed, 80 insertions(+), 80 deletions(-)
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index e6268a8dc378..e64e323244a5 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -512,95 +512,95 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
#undef ERRORS
#define ERRORS \
- C(FILE_NOT_FOUND, "Failed to find the given file"), \
- C(NO_REGULAR_FILE, "Not a regular file"), \
- C(BAD_REFCNT, "Invalid reference counter offset"), \
- C(REFCNT_OPEN_BRACE, "Reference counter brace is not closed"), \
- C(BAD_REFCNT_SUFFIX, "Reference counter has wrong suffix"), \
- C(BAD_UPROBE_OFFS, "Invalid uprobe offset"), \
- C(BAD_MAXACT_TYPE, "Maxactive is only for function exit"), \
- C(BAD_MAXACT, "Invalid maxactive number"), \
- C(MAXACT_TOO_BIG, "Maxactive is too big"), \
- C(BAD_PROBE_ADDR, "Invalid probed address or symbol"), \
- C(NON_UNIQ_SYMBOL, "The symbol is not unique"), \
- C(BAD_RETPROBE, "Retprobe address must be an function entry"), \
- C(NO_TRACEPOINT, "Tracepoint is not found"), \
- C(BAD_TP_NAME, "Invalid character in tracepoint name"),\
- C(BAD_ADDR_SUFFIX, "Invalid probed address suffix"), \
- C(NO_GROUP_NAME, "Group name is not specified"), \
- C(GROUP_TOO_LONG, "Group name is too long"), \
- C(BAD_GROUP_NAME, "Group name must follow the same rules as C identifiers"), \
- C(NO_EVENT_NAME, "Event name is not specified"), \
- C(EVENT_TOO_LONG, "Event name is too long"), \
- C(BAD_EVENT_NAME, "Event name must follow the same rules as C identifiers"), \
- C(EVENT_EXIST, "Given group/event name is already used by another event"), \
- C(RETVAL_ON_PROBE, "$retval is not available on probe"), \
- C(NO_RETVAL, "This function returns 'void' type"), \
- C(BAD_STACK_NUM, "Invalid stack number"), \
+ C(ARGIDX_2BIG, "$argN index is too big"), \
+ C(ARGS_2LONG, "$arg* failed because the argument list is too long"), \
+ C(ARG_NAME_TOO_LONG, "Argument name is too long"), \
+ C(ARG_TOO_LONG, "Argument expression is too long"), \
+ C(ARRAY_NO_CLOSE, "Array is not closed"), \
+ C(ARRAY_TOO_BIG, "Array number is too big"), \
+ C(BAD_ADDR_SUFFIX, "Invalid probed address suffix"), \
+ C(BAD_ARG_NAME, "Argument name must follow the same rules as C identifiers"), \
C(BAD_ARG_NUM, "Invalid argument number"), \
- C(BAD_VAR, "Invalid $-variable specified"), \
- C(BAD_REG_NAME, "Invalid register name"), \
- C(BAD_MEM_ADDR, "Invalid memory address"), \
- C(BAD_IMM, "Invalid immediate value"), \
- C(IMMSTR_NO_CLOSE, "String is not closed with '\"'"), \
- C(FILE_ON_KPROBE, "File offset is not available for kernel probes"), \
- C(BAD_FILE_OFFS, "Invalid file offset value"), \
- C(SYM_ON_UPROBE, "Symbol is not available with uprobe"), \
- C(TOO_MANY_OPS, "Dereference is too much nested"), \
- C(DEREF_NEED_BRACE, "Dereference needs a brace"), \
+ C(BAD_ARRAY_NUM, "Invalid array size"), \
+ C(BAD_ARRAY_SUFFIX, "Array has wrong suffix"), \
+ C(BAD_ATTACH_ARG, "Attached event does not have this field"), \
+ C(BAD_ATTACH_EVENT, "Attached event does not exist"), \
+ C(BAD_BITFIELD, "Invalid bitfield"), \
+ C(BAD_BTF_TID, "Failed to get BTF type info."), \
C(BAD_DEREF_OFFS, "Invalid dereference offset"), \
- C(DEREF_OPEN_BRACE, "Dereference brace is not closed"), \
- C(COMM_CANT_DEREF, "$comm can not be dereferenced"), \
+ C(BAD_EVENT_NAME, "Event name must follow the same rules as C identifiers"), \
C(BAD_FETCH_ARG, "Invalid fetch argument"), \
- C(ARRAY_NO_CLOSE, "Array is not closed"), \
- C(BAD_ARRAY_SUFFIX, "Array has wrong suffix"), \
- C(BAD_ARRAY_NUM, "Invalid array size"), \
- C(ARRAY_TOO_BIG, "Array number is too big"), \
- C(BAD_TYPE, "Unknown type is specified"), \
- C(BAD_STRING, "String accepts only memory argument"), \
+ C(BAD_FILE_OFFS, "Invalid file offset value"), \
+ C(BAD_GROUP_NAME, "Group name must follow the same rules as C identifiers"), \
+ C(BAD_HYPHEN, "Failed to parse single hyphen. Forgot '>'?"), \
+ C(BAD_IMM, "Invalid immediate value"), \
+ C(BAD_INSN_BNDRY, "Probe point is not an instruction boundary"), \
+ C(BAD_MAXACT, "Invalid maxactive number"), \
+ C(BAD_MAXACT_TYPE, "Maxactive is only for function exit"), \
+ C(BAD_MEM_ADDR, "Invalid memory address"), \
+ C(BAD_PROBE_ADDR, "Invalid probed address or symbol"), \
+ C(BAD_REFCNT, "Invalid reference counter offset"), \
+ C(BAD_REFCNT_SUFFIX, "Reference counter has wrong suffix"), \
+ C(BAD_REG_NAME, "Invalid register name"), \
+ C(BAD_RETPROBE, "Retprobe address must be an function entry"), \
+ C(BAD_STACK_NUM, "Invalid stack number"), \
+ C(BAD_STRING, "String accepts only memory argument"), \
C(BAD_SYMSTRING, "Symbol String doesn't accept data/userdata"), \
- C(BAD_BITFIELD, "Invalid bitfield"), \
- C(ARG_NAME_TOO_LONG, "Argument name is too long"), \
- C(NO_ARG_NAME, "Argument name is not specified"), \
- C(BAD_ARG_NAME, "Argument name must follow the same rules as C identifiers"), \
- C(USED_ARG_NAME, "This argument name is already used"), \
- C(ARG_TOO_LONG, "Argument expression is too long"), \
+ C(BAD_TP_NAME, "Invalid character in tracepoint name"), \
+ C(BAD_TYPE, "Unknown type is specified"), \
+ C(BAD_TYPE4STR, "This type does not fit for string."), \
+ C(BAD_UPROBE_OFFS, "Invalid uprobe offset"), \
+ C(BAD_VAR, "Invalid $-variable specified"), \
+ C(BAD_VAR_ARGS, "$arg* must be an independent parameter without name etc."), \
+ C(COMM_CANT_DEREF, "$comm can not be dereferenced"), \
+ C(DEREF_NEED_BRACE, "Dereference needs a brace"), \
+ C(DEREF_OPEN_BRACE, "Dereference brace is not closed"), \
+ C(DIFF_ARG_TYPE, "Argument type or name is different from existing probe"), \
+ C(DIFF_PROBE_TYPE, "Probe type is different from existing probe"), \
+ C(DOUBLE_ARGS, "$arg* can be used only once in the parameters"), \
+ C(EVENT_EXIST, "Given group/event name is already used by another event"), \
+ C(EVENT_TOO_BIG, "Event too big (too many fields?)"), \
+ C(EVENT_TOO_LONG, "Event name is too long"), \
+ C(FAIL_REG_PROBE, "Failed to register probe event"), \
+ C(FILE_NOT_FOUND, "Failed to find the given file"), \
+ C(FILE_ON_KPROBE, "File offset is not available for kernel probes"), \
+ C(GROUP_TOO_LONG, "Group name is too long"), \
+ C(IMMSTR_NO_CLOSE, "String is not closed with '\"'"), \
+ C(MAXACT_TOO_BIG, "Maxactive is too big"), \
+ C(NEED_STRING_TYPE, "$comm and immediate-string only accepts string type"), \
+ C(NOFENTRY_ARGS, "$arg* can be used only on function entry or exit"), \
+ C(NON_UNIQ_SYMBOL, "The symbol is not unique"), \
+ C(NOSUP_BTFARG, "BTF is not available or not supported"), \
+ C(NOSUP_DAT_ARG, "Non pointer structure/union argument is not supported."), \
+ C(NOSUP_PERCPU, "Per-cpu variable access is only for kernel probes"), \
C(NO_ARG_BODY, "No argument expression"), \
- C(BAD_INSN_BNDRY, "Probe point is not an instruction boundary"),\
- C(FAIL_REG_PROBE, "Failed to register probe event"),\
- C(DIFF_PROBE_TYPE, "Probe type is different from existing probe"),\
- C(DIFF_ARG_TYPE, "Argument type or name is different from existing probe"),\
- C(SAME_PROBE, "There is already the exact same probe event"),\
- C(NO_EVENT_INFO, "This requires both group and event name to attach"),\
- C(BAD_ATTACH_EVENT, "Attached event does not exist"),\
- C(BAD_ATTACH_ARG, "Attached event does not have this field"),\
+ C(NO_ARG_NAME, "Argument name is not specified"), \
+ C(NO_BTFARG, "This variable is not found at this probe point"), \
+ C(NO_BTF_ENTRY, "No BTF entry for this probe point"), \
+ C(NO_BTF_FIELD, "This field is not found."), \
C(NO_EP_FILTER, "No filter rule after 'if'"), \
- C(NOSUP_BTFARG, "BTF is not available or not supported"), \
- C(NO_BTFARG, "This variable is not found at this probe point"),\
- C(NO_BTF_ENTRY, "No BTF entry for this probe point"), \
- C(BAD_VAR_ARGS, "$arg* must be an independent parameter without name etc."),\
- C(NOFENTRY_ARGS, "$arg* can be used only on function entry or exit"), \
- C(DOUBLE_ARGS, "$arg* can be used only once in the parameters"), \
- C(ARGS_2LONG, "$arg* failed because the argument list is too long"), \
- C(ARGIDX_2BIG, "$argN index is too big"), \
+ C(NO_EVENT_FIELD, "This event field is not found."), \
+ C(NO_EVENT_INFO, "This requires both group and event name to attach"), \
+ C(NO_EVENT_NAME, "Event name is not specified"), \
+ C(NO_GROUP_NAME, "Group name is not specified"), \
C(NO_PTR_STRCT, "This is not a pointer to union/structure."), \
- C(NOSUP_DAT_ARG, "Non pointer structure/union argument is not supported."),\
- C(BAD_HYPHEN, "Failed to parse single hyphen. Forgot '>'?"), \
- C(NO_EVENT_FIELD, "This event field is not found."), \
- C(NO_BTF_FIELD, "This field is not found."), \
- C(BAD_BTF_TID, "Failed to get BTF type info."),\
- C(BAD_TYPE4STR, "This type does not fit for string."),\
- C(NEED_STRING_TYPE, "$comm and immediate-string only accepts string type"),\
- C(TOO_MANY_ARGS, "Too many arguments are specified"), \
+ C(NO_REGULAR_FILE, "Not a regular file"), \
+ C(NO_RETVAL, "This function returns 'void' type"), \
+ C(NO_TRACEPOINT, "Tracepoint is not found"), \
+ C(REFCNT_OPEN_BRACE, "Reference counter brace is not closed"), \
+ C(RETVAL_ON_PROBE, "$retval is not available on probe"), \
+ C(SAME_PROBE, "There is already the exact same probe event"), \
+ C(SYM_ON_UPROBE, "Symbol is not available with uprobe"), \
+ C(TOO_MANY_ARGS, "Too many arguments are specified"), \
C(TOO_MANY_EARGS, "Too many entry arguments specified"), \
- C(EVENT_TOO_BIG, "Event too big (too many fields?)"), \
- C(TYPECAST_NOT_EVENT, "Typecasts are only for eprobe fields"), \
+ C(TOO_MANY_NESTED, "Too many nested typecasts/dereferences"), \
+ C(TOO_MANY_OPS, "Dereference is too much nested"), \
+ C(TYPECAST_BAD_ARROW, "Typecast field option does not support -> operator"), \
+ C(TYPECAST_NOT_ALIGNED, "Typecast field option is not byte-aligned"), \
+ C(TYPECAST_NOT_EVENT, "Typecasts are only for eprobe fields"), \
C(TYPECAST_REQ_FIELD, "Typecast requires a field access"), \
- C(TOO_MANY_NESTED, "Too many nested typecasts/dereferences"), \
- C(TYPECAST_SYM_OFFSET, "@SYM+/-OFFSET with typecast needs parentheses"), \
- C(TYPECAST_NOT_ALIGNED, "Typecast field option is not byte-aligned"), \
- C(TYPECAST_BAD_ARROW, "Typecast field option does not support -> operator"), \
- C(NOSUP_PERCPU, "Per-cpu variable access is only for kernel probes"),
+ C(TYPECAST_SYM_OFFSET, "@SYM+/-OFFSET with typecast needs parentheses"), \
+ C(USED_ARG_NAME, "This argument name is already used"),
#undef C
#define C(a, b) TP_ERR_##a
^ permalink raw reply related
* [PATCH v3 4/5] tracing/probes: Extend max length of argument string
From: Masami Hiramatsu (Google) @ 2026-07-14 1:10 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
In-Reply-To: <178399136606.27810.4469135881050354189.stgit@devnote2>
From: Masami Hiramatsu <mhiramat@kernel.org>
To support BTF argument parsing (such as accessing fields within nested
structures via typecasting), the maximum argument string length needs
to be extended. Extend MAX_ARGSTR_LEN from 63 to 255.
Since MAX_ARGSTR_LEN was previously reused to format command heads in
trace_*probe_match_command_head() functions, introduce a dedicated
MAX_COMMON_HEAD_LEN (63) macro for matching command heads and switch
these functions to use the new macro.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v2:
- Make the MAX_ARGSTR_LEN to 255 instead of 256.
---
kernel/trace/trace_fprobe.c | 2 +-
kernel/trace/trace_kprobe.c | 2 +-
kernel/trace/trace_probe.h | 3 ++-
kernel/trace/trace_uprobe.c | 2 +-
.../ftrace/test.d/dynevent/fprobe_syntax_errors.tc | 2 +-
.../ftrace/test.d/dynevent/tprobe_syntax_errors.tc | 2 +-
.../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 2 +-
7 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index 536781cd4c47..5638a90e61cc 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -238,7 +238,7 @@ static bool trace_fprobe_is_busy(struct dyn_event *ev)
static bool trace_fprobe_match_command_head(struct trace_fprobe *tf,
int argc, const char **argv)
{
- char buf[MAX_ARGSTR_LEN + 1];
+ char buf[MAX_COMMON_HEAD_LEN + 1];
if (!argc)
return true;
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index cfa807d8e760..cc24e992732c 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -149,7 +149,7 @@ static bool trace_kprobe_is_busy(struct dyn_event *ev)
static bool trace_kprobe_match_command_head(struct trace_kprobe *tk,
int argc, const char **argv)
{
- char buf[MAX_ARGSTR_LEN + 1];
+ char buf[MAX_COMMON_HEAD_LEN + 1];
if (!argc)
return true;
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index e64e323244a5..e6d427910c4f 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -32,7 +32,8 @@
#include "trace_output.h"
#define MAX_TRACE_ARGS 128
-#define MAX_ARGSTR_LEN 63
+#define MAX_ARGSTR_LEN 255
+#define MAX_COMMON_HEAD_LEN 63
#define MAX_ARRAY_LEN 64
#define MAX_ARG_NAME_LEN 32
#define MAX_BTF_ARGS_LEN 128
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index b2e264a4b96c..67bd8fd91e3e 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -281,7 +281,7 @@ static bool trace_uprobe_is_busy(struct dyn_event *ev)
static bool trace_uprobe_match_command_head(struct trace_uprobe *tu,
int argc, const char **argv)
{
- char buf[MAX_ARGSTR_LEN + 1];
+ char buf[MAX_COMMON_HEAD_LEN + 1];
int len;
if (!argc)
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
index e9d7e6919c7f..984ab94df213 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
@@ -75,7 +75,7 @@ check_error 'f vfs_read ^arg123456789012345678901234567890=@11' # ARG_NAME_TOO_L
check_error 'f vfs_read ^=@11' # NO_ARG_NAME
check_error 'f vfs_read ^var.1=@11' # BAD_ARG_NAME
check_error 'f vfs_read var1=@11 ^var1=@12' # USED_ARG_NAME
-check_error 'f vfs_read ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))' # ARG_TOO_LONG
+check_error 'f vfs_read ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))))))))))))))))))))))' # ARG_TOO_LONG
check_error 'f vfs_read arg1=^' # NO_ARG_BODY
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
index ffe8ffef4027..2d0905b2c8b7 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
@@ -61,7 +61,7 @@ check_error 't kfree ^arg123456789012345678901234567890=@11' # ARG_NAME_TOO_LOG
check_error 't kfree ^=@11' # NO_ARG_NAME
check_error 't kfree ^var.1=@11' # BAD_ARG_NAME
check_error 't kfree var1=@11 ^var1=@12' # USED_ARG_NAME
-check_error 't kfree ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))' # ARG_TOO_LONG
+check_error 't kfree ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))))))))))))))))))))))' # ARG_TOO_LONG
check_error 't kfree arg1=^' # NO_ARG_BODY
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
index 21ce8414459f..d28f63b7e8a9 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
@@ -71,7 +71,7 @@ check_error 'p vfs_read ^arg123456789012345678901234567890=@11' # ARG_NAME_TOO_L
check_error 'p vfs_read ^=@11' # NO_ARG_NAME
check_error 'p vfs_read ^var.1=@11' # BAD_ARG_NAME
check_error 'p vfs_read var1=@11 ^var1=@12' # USED_ARG_NAME
-check_error 'p vfs_read ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))' # ARG_TOO_LONG
+check_error 'p vfs_read ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))))))))))))))))))))))' # ARG_TOO_LONG
check_error 'p vfs_read arg1=^' # NO_ARG_BODY
# instruction boundary check is valid on x86 (at this moment)
^ permalink raw reply related
* [PATCH v3 5/5] tracing/probes: Eliminate recursion in parse_probe_arg()
From: Masami Hiramatsu (Google) @ 2026-07-14 1:10 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
In-Reply-To: <178399136606.27810.4469135881050354189.stgit@devnote2>
From: Masami Hiramatsu <mhiramat@kernel.org>
To avoid potential stack overflows on limited kernel stacks, convert
parse_probe_arg() from a recursive function into a loop-based
implementation with a simple local state stack.
Since recursion is eliminated using a loop with a fixed-size
stack in the context, this restricts the dereference nesting
depth. The maximum nesting depth of dereferences is now restricted
to the same limit as typecasts (TRACEPROBE_MAX_NESTED_LEVEL, which
is 8) and reports the same TOO_MANY_NESTED error. Update ftrace
selftests to reflect this restriction and simplify the checks.
Note that this change slightly alters the behavior of nested
dereferencing in fetcharg. Previously, dereferencing without BTF
allowed for up to 14 levels of nesting, whereas dereferencing
with BTF was limited to 3 levels. With this change, the nesting
depth is now limited to 8 levels in both cases.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v3:
- Removed unused is_ptr.
- Add trace_probe_log_err() to the error case in parse_this_cpu().
Changes in v2:
- Clearly state that both derefernce nest and typecast nest are
limited by the same TRACEPROBE_MAX_NESTED_LEVEL.
- Extend TRACEPROBE_MAX_NESTED_LEVEL to 8.
- Fix testcase to check TOO_MANY_NESTED instead of TOO_MANY_OPS
(that should not be hit anymore)
---
kernel/trace/trace_probe.c | 338 ++++++++++++--------
kernel/trace/trace_probe.h | 33 ++
.../ftrace/test.d/dynevent/fprobe_syntax_errors.tc | 4
.../ftrace/test.d/dynevent/tprobe_syntax_errors.tc | 2
.../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 4
5 files changed, 242 insertions(+), 139 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index eb9f7bd13c19..7568f5e68de7 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -356,61 +356,10 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
struct fetch_insn **pcode, struct fetch_insn *end,
struct traceprobe_parse_context *ctx);
-/* handle dereference nested call */
-static inline int handle_dereference(char *arg, struct fetch_insn **pcode,
- struct fetch_insn *end, struct traceprobe_parse_context *ctx,
- int deref, long offset)
+static int parse_this_cpu(char *arg, struct traceprobe_parse_context *ctx)
{
- const struct fetch_type *type = find_fetch_type(NULL, ctx->flags);
- struct fetch_insn *code = *pcode;
- int cur_offs = ctx->offset;
+ bool is_read = false;
char *tmp;
- int ret;
-
- tmp = strrchr(arg, ')');
- if (!tmp) {
- trace_probe_log_err(ctx->offset + strlen(arg),
- DEREF_OPEN_BRACE);
- return -EINVAL;
- }
-
- *tmp = '\0';
- ret = parse_probe_arg(arg, type, &code, end, ctx);
- if (ret)
- return ret;
- ctx->offset = cur_offs;
- if (code->op == FETCH_OP_COMM || code->op == FETCH_OP_IMMSTR) {
- trace_probe_log_err(ctx->offset, COMM_CANT_DEREF);
- return -EINVAL;
- }
-
- /*
- * this_cpu_ptr(@SYM) does not use SYM value, but use SYM address.
- * So we overwrite the last FETCH_OP_DEREF with FETCH_OP_CPU_PTR.
- */
- if (!(deref == FETCH_OP_CPU_PTR && *arg == '@')) {
- code++;
- if (code == end) {
- trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
- return -EINVAL;
- }
- }
- *pcode = code;
-
- code->op = deref;
- code->offset = offset;
- /* Reset the last type if used */
- ctx->last_type = NULL;
- return 0;
-}
-
-static int parse_this_cpu(char *arg, struct fetch_insn **pcode,
- struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
-{
- struct fetch_insn *code;
- bool is_ptr = false;
- int ret;
/*
* This is only for kernel probes, excluding eprobe, because per-cpu
@@ -424,27 +373,29 @@ static int parse_this_cpu(char *arg, struct fetch_insn **pcode,
if (str_has_prefix(arg, THIS_CPU_PTR_PREFIX)) {
arg += THIS_CPU_PTR_LEN;
ctx->offset += THIS_CPU_PTR_LEN;
- is_ptr = true;
} else if (str_has_prefix(arg, THIS_CPU_READ_PREFIX)) {
arg += THIS_CPU_READ_LEN;
ctx->offset += THIS_CPU_READ_LEN;
- } else
+ is_read = true;
+ } else {
+ trace_probe_log_err(ctx->offset, BAD_FETCH_ARG);
return -EINVAL;
+ }
- ret = handle_dereference(arg, pcode, end, ctx, FETCH_OP_CPU_PTR, 0);
- if (ret || is_ptr)
- return ret;
-
- /* this_cpu_read(VAR) -> +0(this_cpu_ptr(VAR)) */
- code = *pcode;
- code++;
- if (code == end) {
- trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ tmp = strrchr(arg, ')');
+ if (!tmp) {
+ trace_probe_log_err(ctx->offset + strlen(arg),
+ DEREF_OPEN_BRACE);
return -EINVAL;
}
- code->op = FETCH_OP_DEREF;
- code->offset = 0;
- *pcode = code;
+ *tmp = '\0';
+
+ ctx->stack[ctx->depth].type = STATE_DEREF;
+ ctx->stack[ctx->depth].deref.deref = FETCH_OP_CPU_PTR;
+ ctx->stack[ctx->depth].deref.offset = 0;
+ ctx->stack[ctx->depth].deref.cur_offs = ctx->offset;
+ ctx->stack[ctx->depth].deref.inner_arg = arg;
+ ctx->stack[ctx->depth].deref.is_cpu_read = is_read;
return 0;
}
@@ -1007,14 +958,12 @@ static char *find_matched_close_paren(char *s)
return NULL;
}
-static int handle_typecast(char *arg, struct fetch_insn **pcode,
- struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
+static int handle_typecast(char *arg, struct traceprobe_parse_context *ctx)
{
int orig_offset = ctx->offset;
char *close;
char *tmp;
- int ret;
+ char *fieldname;
if (!(tparg_is_event_probe(ctx->flags) ||
tparg_is_function_entry(ctx->flags) ||
@@ -1028,12 +977,6 @@ static int handle_typecast(char *arg, struct fetch_insn **pcode,
* For example: (STRUCT)VAR->FIELD and (STRUCT)(VAR)->FIELD are same.
* VAR is solved in the nested call.
*/
- ctx->nested_level++;
- if (ctx->nested_level > TRACEPROBE_MAX_NESTED_LEVEL) {
- trace_probe_log_err(ctx->offset, TOO_MANY_NESTED);
- return -E2BIG;
- }
-
tmp = strchr(arg, ')');
if (!tmp) {
trace_probe_log_err(ctx->offset + strlen(arg),
@@ -1103,30 +1046,19 @@ static int handle_typecast(char *arg, struct fetch_insn **pcode,
}
*close = '\0';
- /* We need to parse the nested one */
- ret = parse_probe_arg(tmp, find_fetch_type(NULL, ctx->flags),
- pcode, end, ctx);
- if (ret < 0)
- return ret;
- ctx->nested_level--;
- clear_struct_btf(ctx);
-
- /* Let tmp point the field name. */
+ /* Let fieldname point the field name. */
if (close[1] == '-')
- tmp = close + 3; /* Skip "->" after closing parenthesis */
+ fieldname = close + 3; /* Skip "->" after closing parenthesis */
else
- tmp = close + 2; /* Skip ">" after inner variable name */
-
- /* resolve the typecast struct name */
- ctx->offset = orig_offset + 1; /* for the '(' */
- ret = parse_btf_casttype(arg + 1, ctx);
- if (ret < 0)
- return ret;
-
- ctx->offset = orig_offset + tmp - arg;
- ret = parse_btf_field(tmp, ctx->last_struct, pcode, end, ctx);
- ctx->prefix_byteoffs = 0;
- return ret;
+ fieldname = close + 2; /* Skip ">" after inner variable name */
+
+ ctx->stack[ctx->depth].type = STATE_TYPECAST;
+ ctx->stack[ctx->depth].typecast.casttype = arg + 1;
+ ctx->stack[ctx->depth].typecast.fieldname = fieldname;
+ ctx->stack[ctx->depth].typecast.orig_offset = orig_offset;
+ ctx->stack[ctx->depth].typecast.field_offset_diff = fieldname - arg;
+ ctx->stack[ctx->depth].typecast.inner_arg = tmp;
+ return 0;
}
#else /* !CONFIG_PROBE_EVENTS_BTF_ARGS */
@@ -1171,9 +1103,20 @@ static int check_prepare_btf_string_fetch(char *typename,
return 0;
}
-static int handle_typecast(char *arg, struct fetch_insn **pcode,
- struct fetch_insn *end,
+static int parse_btf_casttype(char *casttype,
+ struct traceprobe_parse_context *ctx)
+{
+ return -EOPNOTSUPP;
+}
+
+static int parse_btf_field(char *fieldname, const struct btf_type *type,
+ struct fetch_insn **pcode, struct fetch_insn *end,
struct traceprobe_parse_context *ctx)
+{
+ return -EOPNOTSUPP;
+}
+
+static int handle_typecast(char *arg, struct traceprobe_parse_context *ctx)
{
trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
return -EOPNOTSUPP;
@@ -1598,9 +1541,7 @@ static int parse_probe_arg_mem_symbol(char *arg, struct fetch_insn **pcode,
return 0;
}
-static int parse_probe_arg_deref(char *arg, struct fetch_insn **pcode,
- struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
+static int parse_probe_arg_deref(char *arg, struct traceprobe_parse_context *ctx)
{
int deref = FETCH_OP_DEREF;
long offset = 0;
@@ -1627,7 +1568,22 @@ static int parse_probe_arg_deref(char *arg, struct fetch_insn **pcode,
}
ctx->offset += (tmp + 1 - arg) + (arg[0] != '-' ? 1 : 0);
arg = tmp + 1;
- return handle_dereference(arg, pcode, end, ctx, deref, offset);
+
+ tmp = strrchr(arg, ')');
+ if (!tmp) {
+ trace_probe_log_err(ctx->offset + strlen(arg),
+ DEREF_OPEN_BRACE);
+ return -EINVAL;
+ }
+ *tmp = '\0';
+
+ ctx->stack[ctx->depth].type = STATE_DEREF;
+ ctx->stack[ctx->depth].deref.deref = deref;
+ ctx->stack[ctx->depth].deref.offset = offset;
+ ctx->stack[ctx->depth].deref.cur_offs = ctx->offset;
+ ctx->stack[ctx->depth].deref.inner_arg = arg;
+ ctx->stack[ctx->depth].deref.is_cpu_read = false;
+ return 0;
}
static int parse_probe_arg_imm(char *arg, struct fetch_insn *code,
@@ -1659,11 +1615,6 @@ static int parse_probe_arg_default(char *arg, struct fetch_insn **pcode,
{
int ret;
- if (str_has_prefix(arg, THIS_CPU_PTR_PREFIX) ||
- str_has_prefix(arg, THIS_CPU_READ_PREFIX)) {
- return parse_this_cpu(arg, pcode, end, ctx);
- }
-
if (isalpha(arg[0]) || arg[0] == '_') {
/* BTF variable or event field */
if (ctx->flags & TPARG_FL_TEVENT) {
@@ -1685,11 +1636,56 @@ static int parse_probe_arg_default(char *arg, struct fetch_insn **pcode,
return 0;
}
-/* Recursive argument parser */
-static int
-parse_probe_arg(char *arg, const struct fetch_type *type,
- struct fetch_insn **pcode, struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
+static int parse_probe_arg_nested(char **parg, struct traceprobe_parse_context *ctx)
+{
+ char *arg = *parg;
+ int ret;
+
+ while (true) {
+ /* Determine if this is a nested argument */
+ if (arg[0] != '+' && arg[0] != '-' && arg[0] != '(' &&
+ !str_has_prefix(arg, THIS_CPU_PTR_PREFIX) &&
+ !str_has_prefix(arg, THIS_CPU_READ_PREFIX))
+ break;
+
+ /* If nested, check the maximum depth limit */
+ if (ctx->depth >= TRACEPROBE_MAX_NESTED_LEVEL) {
+ trace_probe_log_err(ctx->offset, TOO_MANY_NESTED);
+ return -E2BIG;
+ }
+
+ /* Perform the actual parsing subroutine calls */
+ switch (arg[0]) {
+ case '+':
+ case '-':
+ ret = parse_probe_arg_deref(arg, ctx);
+ if (ret)
+ return ret;
+ arg = ctx->stack[ctx->depth].deref.inner_arg;
+ break;
+ case '(':
+ ret = handle_typecast(arg, ctx);
+ if (ret)
+ return ret;
+ arg = ctx->stack[ctx->depth].typecast.inner_arg;
+ break;
+ default:
+ ret = parse_this_cpu(arg, ctx);
+ if (ret)
+ return ret;
+ arg = ctx->stack[ctx->depth].deref.inner_arg;
+ break;
+ }
+ ctx->depth++;
+ }
+
+ *parg = arg;
+ return 0;
+}
+
+static int parse_probe_arg_leaf(char *arg, const struct fetch_type *type,
+ struct fetch_insn **pcode, struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
{
struct fetch_insn *code = *pcode;
int ret;
@@ -1704,26 +1700,112 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
case '@': /* memory, file-offset or symbol */
ret = parse_probe_arg_mem_symbol(arg, pcode, end, ctx);
break;
- case '+': /* deref memory */
- case '-':
- ret = parse_probe_arg_deref(arg, pcode, end, ctx);
- break;
case '\\': /* Immediate value */
ret = parse_probe_arg_imm(arg, code, ctx);
break;
- case '(':
- ret = handle_typecast(arg, pcode, end, ctx);
- break;
default:
ret = parse_probe_arg_default(arg, pcode, end, ctx);
break;
}
- if (!ret && code->op == FETCH_OP_NOP) {
+
+ if (ret)
+ return ret;
+
+ if (code->op == FETCH_OP_NOP) {
/* Parsed, but do not find fetch method */
trace_probe_log_err(ctx->offset, BAD_FETCH_ARG);
- ret = -EINVAL;
+ return -EINVAL;
}
- return ret;
+
+ return 0;
+}
+
+static int unwind_parse_states(struct fetch_insn **pcode, struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ struct parse_state *state;
+ struct fetch_insn *code;
+ int ret;
+
+ while (ctx->depth > 0) {
+ ctx->depth--;
+ state = &ctx->stack[ctx->depth];
+
+ if (state->type == STATE_DEREF) {
+ code = *pcode;
+ ctx->offset = state->deref.cur_offs;
+ if (code->op == FETCH_OP_COMM || code->op == FETCH_OP_IMMSTR) {
+ trace_probe_log_err(ctx->offset, COMM_CANT_DEREF);
+ return -EINVAL;
+ }
+
+ if (!(state->deref.deref == FETCH_OP_CPU_PTR &&
+ *state->deref.inner_arg == '@')) {
+ code++;
+ if (code == end) {
+ trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ return -EINVAL;
+ }
+ }
+ *pcode = code;
+
+ code->op = state->deref.deref;
+ code->offset = state->deref.offset;
+ ctx->last_type = NULL;
+
+ if (state->deref.is_cpu_read) {
+ code = *pcode;
+ code++;
+ if (code == end) {
+ trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ return -EINVAL;
+ }
+ code->op = FETCH_OP_DEREF;
+ code->offset = 0;
+ *pcode = code;
+ }
+ } else if (state->type == STATE_TYPECAST) {
+ clear_struct_btf(ctx);
+
+ /* resolve the typecast struct name */
+ ctx->offset = state->typecast.orig_offset + 1; /* for the '(' */
+ ret = parse_btf_casttype(state->typecast.casttype, ctx);
+ if (ret < 0)
+ return ret;
+
+ ctx->offset = state->typecast.orig_offset +
+ state->typecast.field_offset_diff;
+ ret = parse_btf_field(state->typecast.fieldname,
+ ctx->last_struct, pcode,
+ end, ctx);
+ ctx->prefix_byteoffs = 0;
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+/* Loop-based (non-recursive) argument parser */
+static int
+parse_probe_arg(char *arg, const struct fetch_type *type,
+ struct fetch_insn **pcode, struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ int ret;
+
+ ctx->depth = 0;
+
+ ret = parse_probe_arg_nested(&arg, ctx);
+ if (ret)
+ return ret;
+
+ ret = parse_probe_arg_leaf(arg, type, pcode, end, ctx);
+ if (ret)
+ return ret;
+
+ return unwind_parse_states(pcode, end, ctx);
}
/* Bitfield type needs to be parsed into a fetch function */
@@ -1984,12 +2066,6 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
ctx);
if (ret < 0)
goto fail;
- /* nested_level must be 0 here, otherwise there is a bug. */
- if (WARN_ON_ONCE(ctx->nested_level)) {
- ret = -EINVAL;
- goto fail;
- }
-
/* Update storing type if BTF is available */
if (IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS) &&
ctx->last_type) {
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index e6d427910c4f..ebdc706e7cb6 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -437,6 +437,34 @@ static inline bool tparg_is_event_probe(unsigned int flags)
return !!(flags & TPARG_FL_TEVENT);
}
+/* Each typecast consumes nested level. So the max number of typecast is 8. */
+#define TRACEPROBE_MAX_NESTED_LEVEL 8
+
+enum parse_state_type {
+ STATE_DEREF,
+ STATE_TYPECAST,
+};
+
+struct parse_state {
+ int type;
+ union {
+ struct {
+ int deref;
+ long offset;
+ int cur_offs;
+ char *inner_arg;
+ bool is_cpu_read;
+ } deref;
+ struct {
+ char *casttype;
+ char *fieldname;
+ int orig_offset;
+ int field_offset_diff;
+ char *inner_arg;
+ } typecast;
+ };
+};
+
struct traceprobe_parse_context {
struct trace_event_call *event;
/* BTF related parameters */
@@ -453,12 +481,11 @@ struct traceprobe_parse_context {
struct trace_probe *tp;
unsigned int flags;
int offset;
- int nested_level;
int prefix_byteoffs; /* The byte offset of the prefix field of typecast */
+ struct parse_state stack[TRACEPROBE_MAX_NESTED_LEVEL + 1];
+ int depth;
};
-/* Each typecast consumes nested level. So the max number of typecast is 3. */
-#define TRACEPROBE_MAX_NESTED_LEVEL 3
extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
const char *argv,
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
index 984ab94df213..384209968325 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
@@ -60,7 +60,7 @@ check_error 'f vfs_read ^&1' # BAD_FETCH_ARG
# We've introduced this limitation with array support
if grep -q ' <type>\\\[<array-size>\\\]' README; then
-check_error 'f vfs_read +0(^+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(@0))))))))))))))' # TOO_MANY_OPS?
+check_error 'f vfs_read +0(+0(+0(+0(+0(+0(+0(+0(^+0(@0)))))))))' # TOO_MANY_NESTED
check_error 'f vfs_read +0(@11):u8[10^' # ARRAY_NO_CLOSE
check_error 'f vfs_read +0(@11):u8[10]^a' # BAD_ARRAY_SUFFIX
check_error 'f vfs_read +0(@11):u8[^10a]' # BAD_ARRAY_NUM
@@ -114,7 +114,7 @@ check_error 'f vfs_read file^-.foo' # BAD_HYPHEN
check_error 'f vfs_read ^file:string' # BAD_TYPE4STR
if grep -qF "[(structname" README ; then
check_error 'f vfs_read arg1=(task_struct)file^' # TYPECAST_REQ_FIELD
-check_error 'f vfs_read arg1=(a)((b)((c)(^(d)file->d)->c)->b)->a' # TOO_MANY_NESTED
+check_error 'f vfs_read arg1=(a)((b)((c)((d)((e)((f)((g)((h)(^(i)file->i)->h)->g)->f)->e)->d)->c)->b)->a' # TOO_MANY_NESTED
check_error 'f vfs_read arg1=(task_struct,^in_execve)file->comm' # TYPECAST_NOT_ALIGNED
check_error 'f vfs_read arg1=(task_struct,^foo_bar)file->pid' # NO_BTF_FIELD
check_error 'f vfs_read arg1=(^task_struct1234)file->pid' # NO_PTR_STRCT
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
index 2d0905b2c8b7..72b8652df9ba 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
@@ -46,7 +46,7 @@ check_error 't kfree ^&1' # BAD_FETCH_ARG
# We've introduced this limitation with array support
if grep -q ' <type>\\\[<array-size>\\\]' README; then
-check_error 't kfree +0(^+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(@0))))))))))))))' # TOO_MANY_OPS?
+check_error 't kfree +0(+0(+0(+0(+0(+0(+0(+0(^+0(@0)))))))))' # TOO_MANY_NESTED
check_error 't kfree +0(@11):u8[10^' # ARRAY_NO_CLOSE
check_error 't kfree +0(@11):u8[10]^a' # BAD_ARRAY_SUFFIX
check_error 't kfree +0(@11):u8[^10a]' # BAD_ARRAY_NUM
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
index d28f63b7e8a9..b0e6b80ccb01 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
@@ -56,7 +56,7 @@ check_error 'p vfs_read ^&1' # BAD_FETCH_ARG
# We've introduced this limitation with array support
if grep -q ' <type>\\\[<array-size>\\\]' README; then
-check_error 'p vfs_read +0(^+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(@0))))))))))))))' # TOO_MANY_OPS?
+check_error 'p vfs_read +0(+0(+0(+0(+0(+0(+0(+0(^+0(@0)))))))))' # TOO_MANY_NESTED
check_error 'p vfs_read +0(@11):u8[10^' # ARRAY_NO_CLOSE
check_error 'p vfs_read +0(@11):u8[10]^a' # BAD_ARRAY_SUFFIX
check_error 'p vfs_read +0(@11):u8[^10a]' # BAD_ARRAY_NUM
@@ -117,7 +117,7 @@ check_error 'p kfree ^$arg10' # NO_BTFARG (exceed the number of parameters)
check_error 'r kfree ^$retval' # NO_RETVAL
if grep -qF "[(structname" README ; then
check_error 'p vfs_read arg1=(task_struct)file^' # TYPECAST_REQ_FIELD
-check_error 'p vfs_read arg1=(a)((b)((c)(^(d)file->d)->c)->b)->a' # TOO_MANY_NESTED
+check_error 'p vfs_read arg1=(a)((b)((c)((d)((e)((f)((g)((h)(^(i)file->i)->h)->g)->f)->e)->d)->c)->b)->a' # TOO_MANY_NESTED
check_error 'p vfs_read arg1=(task_struct,^in_execve)file->comm' # TYPECAST_NOT_ALIGNED
check_error 'p vfs_read arg1=(task_struct,^foo_bar)file->pid' # NO_BTF_FIELD
check_error 'p vfs_read arg1=(^task_struct1234)file->pid' # NO_PTR_STRCT
^ permalink raw reply related
* Re: [PATCH v2 1/4] mm/migrate: do not migrate folios mapped into VM_LOCKED VMAs under compaction
From: Wandun @ 2026-07-14 1:45 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: vbabka, david, rostedt, mhiramat, Alexander.Krabler, hughd, fvdl,
bigeasy, linux-mm, linux-kernel, linux-trace-kernel,
linux-rt-devel, akpm, surenb, mhocko, jackmanb, hannes, ziy, riel,
liam, harry, jannh, lance.yang, mathieu.desnoyers, matthew.brost,
joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple,
pfalcato
In-Reply-To: <ak-2VAwNNuCD98Jv@lucifer>
On 7/9/26 23:00, Lorenzo Stoakes wrote:
> On Thu, Jul 09, 2026 at 04:25:27PM +0800, Wandun wrote:
>>
>>
>> On 7/7/26 21:55, Lorenzo Stoakes wrote:
>>> On Tue, Jul 07, 2026 at 02:44:50PM +0100, Lorenzo Stoakes wrote:
>>>> 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.
>>>>
>>>> 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?
>>>
>>> Also, since compaction_allow_unevictable() is a function that is accessible
>>> elsewhere, you could literally just have a TTU_MIGRATION here instead and have
>>> the rmap logic call compaction_allow_unevictable() instead rather than this.
>> Do you mean:
>> 1. change TTU_RESPECT_MLOCK to TTU_MIGRATION, that'is OK.
>> 2. move call compaction_allow_unevictable() to try_to_migrate_one? but as you suggested
>> migrate_mlock_allowed function, it already called compaction_allow_unevictable()
>> in order to determine whether TTU_MIGRATION needs to be added. Did I misunderstand
>> something somewhere?
>
> The next paragraph addresses this.
>
>>
>>>
>>> And then you could adapt the function I suggested before not to take a reason
>>> parameter but rather a 'is_migration' one instead possibly and then pass (ttu &
>>> TTU_MIGRATION) in.
>
>
>>>
>>> BUT. I still question whether this is at all needed since you have the folio you
>>> can check for PG_mlocked...
>>
>> I described a race scenario at this link:
>> https://lore.kernel.org/lkml/c372e68f-2bfc-45b6-a431-01bf55717247@gmail.com/
>
> OK thanks. You really have to spell this out in the code in a comment, this is
> really non-obvious.
I will address these in the next version.
Thanks,
Wandun
>
> I will try to look at your reply in due course (am between jobs atm).
>
>>
>>
>>>
>>> Cheers, Lorenzo
>>
>
> Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH] tracing: Add mutex to trace_parser to fix concurrent write races
From: Tengda Wu @ 2026-07-14 1:50 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel
In-Reply-To: <20260713153624.48fc4be4@robin>
Hi Steven,
On 2026/7/14 3:36, Steven Rostedt wrote:
> On Mon, 13 Jul 2026 13:46:40 +0000
> Tengda Wu <wutengda@huaweicloud.com> wrote:
>
>> The trace_parser structure is allocated and initialized when a trace
>> file is opened, and is subsequently used in the write handler to parse
>> user input. If userspace opens a trace file descriptor and shares it
>> across multiple threads, concurrent write calls will race on the
>> parser's internal state, specifically the idx, cont, and buffer fields,
>> leading to corrupted input or undefined behavior.
>>
>> Fix this by embedding a mutex directly in struct trace_parser. The mutex
>> is initialized in trace_parser_get_init() and destroyed in
>> trace_parser_put(). All write-side users that access parser state
>> (trace_get_user() followed by checking trace_parser_loaded() /
>> trace_parser_cont() against the buffer) now hold the mutex across the
>> full critical section, avoiding any TOCTOU gap between the parse and the
>> subsequent consumption of parser->buffer.
>>
>> Affected write paths:
>> - ftrace_graph_write / ftrace_graph_release
>> - ftrace_regex_write / ftrace_regex_release
>>
>> Fixes: e704eff3ff51 ("ftrace: Have set_graph_function handle multiple functions in one write")
>> Fixes: 689fd8b65d66 ("tracing: trace parser support for function and graph")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
>> ---
>> kernel/trace/ftrace.c | 7 +++++++
>> kernel/trace/trace.c | 2 ++
>> kernel/trace/trace.h | 1 +
>> 3 files changed, 10 insertions(+)
>>
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index f93e34dd2328..ef47e5659283 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -5842,6 +5842,8 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
>> /* iter->hash is a local copy, so we don't need regex_lock */
>>
>> parser = &iter->parser;
>> +
>> + guard(mutex)(&parser->lock);
>> read = trace_get_user(parser, ubuf, cnt, ppos);
>
> Why are the other users of trace_get_user() not a problem? If
> anything, trace_get_user() should have a lockdep assert to make sure
> the lock is held.
>
> I think we need to add a lockdep assertion in all the callers that use
> the parser and we need to make sure it's taken by every user.
>
> -- Steve
Other places that use trace_get_user():
ftrace_event_write
trace_parser_get_init
trace_get_user
trace_pid_write
trace_parser_get_init
trace_get_user
In both of these cases, the parser is allocated via trace_parser_get_init()
before use and freed immediately after, with no multi-threaded sharing.
However, ftrace_graph_write() and ftrace_regex_write() are different.
The parser used by these two functions is allocated at open() time and
retrieved from struct file at write() time. Userspace can have multiple
threads invoking write() concurrently, e.g.:
r0 = openat(AT_FDCWD, path, O_WRONLY | O_CREAT | O_TRUNC |
O_NOCTTY | O_NONBLOCK, 0);
r1 = dup(r0);
write(r1, data1, len1); /* thread 1 */
pwrite64(r1, data2, len2, offset); /* thread 2 */
Without proper synchronization in trace_get_user(), the parser state
becomes undefined.
We have locally reproduced a slab-out-of-bounds issue with syzkaller [1],
which appears to be caused by this race.
Regarding the fix, adding a lockdep assertion to trace_get_user() does
not seem necessary. As mentioned above, ftrace_event_write() and
trace_pid_write() do not involve concurrency, so they do not require
locking, and thus do not need a lockdep assertion.
[1] slab-out-of-bounds call trace:
[ 29.629421][ T1269] ==================================================================
[ 29.630348][ T1269] BUG: KASAN: slab-out-of-bounds in strstr+0x44/0x190
[ 29.631098][ T1269] Read of size 1 at addr ffff00000ffa95f3 by task syz.3.4/1269
[ 29.632120][ T1269]
[ 29.632390][ T1269] CPU: 2 PID: 1269 Comm: syz.3.4 Not tainted 5.10.0-27349-g9a396a20c0b1-dirty #1
[ 29.634205][ T1269] Hardware name: linux,dummy-virt (DT)
[ 29.635862][ T1269] Call trace:
[ 29.636949][ T1269] dump_backtrace+0x0/0x510
[ 29.637954][ T1269] show_stack+0x34/0x44
[ 29.638438][ T1269] dump_stack+0x1d0/0x248
[ 29.639006][ T1269] print_address_description.constprop.0+0x2c/0x1fc
[ 29.639776][ T1269] __kasan_report+0xfc/0x160
[ 29.640325][ T1269] kasan_report+0x44/0x60
[ 29.640904][ T1269] __asan_load1+0x70/0xa0
[ 29.641544][ T1269] strstr+0x44/0x190
[ 29.642074][ T1269] ftrace_match+0x130/0x200
[ 29.642662][ T1269] ftrace_match_record.isra.0+0x16c/0x210
[ 29.643317][ T1269] ftrace_graph_set_hash+0x20c/0x440
[ 29.643959][ T1269] ftrace_graph_write+0x170/0x200
[ 29.644534][ T1269] vfs_write+0x17c/0x66c
[ 29.645018][ T1269] ksys_write+0x108/0x220
[ 29.645510][ T1269] __arm64_sys_write+0x54/0x70
[ 29.646058][ T1269] el0_svc_common.constprop.0+0xd8/0x37c
[ 29.646683][ T1269] do_el0_svc+0x50/0x130
[ 29.647191][ T1269] el0_svc+0x24/0x34
[ 29.647648][ T1269] el0_sync_handler+0x180/0x18c
[ 29.648232][ T1269] el0_sync+0x160/0x180
[ 29.648712][ T1269]
[ 29.648994][ T1269] Allocated by task 1269:
[ 29.649505][ T1269] kasan_save_stack+0x28/0x60
[ 29.650069][ T1269] __kasan_kmalloc.constprop.0+0xd8/0x140
[ 29.650794][ T1269] kasan_kmalloc+0x10/0x20
[ 29.651365][ T1269] __kmalloc+0xd4/0x7c0
[ 29.651902][ T1269] trace_parser_get_init+0x3c/0xa0
[ 29.652547][ T1269] __ftrace_graph_open.part.0+0xc4/0x250
[ 29.653178][ T1269] ftrace_graph_open+0xd8/0x154
[ 29.653715][ T1269] do_dentry_open+0x400/0x950
[ 29.654231][ T1269] vfs_open+0x60/0x70
[ 29.654681][ T1269] do_open+0x4f0/0x6f0
[ 29.655223][ T1269] path_openat+0x268/0x3a4
[ 29.655707][ T1269] do_filp_open+0x10c/0x23c
[ 29.656234][ T1269] do_sys_openat2+0x4e4/0x700
[ 29.656749][ T1269] __arm64_sys_openat+0x11c/0x1a0
[ 29.657335][ T1269] el0_svc_common.constprop.0+0xd8/0x37c
[ 29.657342][ T1269] do_el0_svc+0x50/0x130
[ 29.657350][ T1269] el0_svc+0x24/0x34
[ 29.657359][ T1269] el0_sync_handler+0x180/0x18c
[ 29.657366][ T1269] el0_sync+0x160/0x180
[ 29.657368][ T1269]
[ 29.657375][ T1269] The buggy address belongs to the object at ffff00000ffa9500
[ 29.657375][ T1269] which belongs to the cache kmalloc-256 of size 256
[ 29.657383][ T1269] The buggy address is located 243 bytes inside of
[ 29.657383][ T1269] 256-byte region [ffff00000ffa9500, ffff00000ffa9600)
[ 29.657386][ T1269] The buggy address belongs to the page:
[ 29.657398][ T1269] page:fffffe00001fea00 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff00000ffa8500 pfn:0x4ffa8
[ 29.657405][ T1269] head:fffffe00001fea00 order:2 compound_mapcount:0 compound_pincount:0
[ 29.657417][ T1269] flags: 0x7ffff800010200(slab|head|node=0|zone=0|lastcpupid=0xfffff)
[ 29.657430][ T1269] raw: 007ffff800010200 ffff0000c0000820 ffff0000c0000820 ffff0000c000cd00
[ 29.657441][ T1269] raw: ffff00000ffa8500 0000000000100008 00000001ffffffff 0000000000000000
[ 29.657445][ T1269] page dumped because: kasan: bad access detected
[ 29.657447][ T1269]
[ 29.657451][ T1269] Memory state around the buggy address:
[ 29.657458][ T1269] ffff00000ffa9480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 29.657465][ T1269] ffff00000ffa9500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 29.657473][ T1269] >ffff00000ffa9580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 03 fc
[ 29.657477][ T1269] ^
[ 29.657484][ T1269] ffff00000ffa9600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 29.657491][ T1269] ffff00000ffa9680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 29.657494][ T1269] ==================================================================
[ 29.657498][ T1269] Disabling lock debugging due to kernel taint
[ 29.657656][ T1269] Kernel panic - not syncing: KASAN: panic_on_warn set ...
[ 29.657668][ T1269] CPU: 2 PID: 1269 Comm: syz.3.4 Tainted: G B 5.10.0-27349-g9a396a20c0b1-dirty #1
[ 29.657672][ T1269] Hardware name: linux,dummy-virt (DT)
[ 29.657676][ T1269] Call trace:
[ 29.657685][ T1269] dump_backtrace+0x0/0x510
[ 29.657692][ T1269] show_stack+0x34/0x44
[ 29.657700][ T1269] dump_stack+0x1d0/0x248
[ 29.657711][ T1269] panic+0x390/0x728
[ 29.657719][ T1269] check_panic_on_warn+0x108/0x12c
[ 29.657727][ T1269] end_report+0x5c/0xe0
[ 29.657748][ T1269] __kasan_report+0x118/0x160
[ 29.657756][ T1269] kasan_report+0x44/0x60
[ 29.657764][ T1269] __asan_load1+0x70/0xa0
[ 29.657771][ T1269] strstr+0x44/0x190
[ 29.657780][ T1269] ftrace_match+0x130/0x200
[ 29.657790][ T1269] ftrace_match_record.isra.0+0x16c/0x210
[ 29.657799][ T1269] ftrace_graph_set_hash+0x20c/0x440
[ 29.657808][ T1269] ftrace_graph_write+0x170/0x200
[ 29.657815][ T1269] vfs_write+0x17c/0x66c
[ 29.657821][ T1269] ksys_write+0x108/0x220
[ 29.657828][ T1269] __arm64_sys_write+0x54/0x70
[ 29.657836][ T1269] el0_svc_common.constprop.0+0xd8/0x37c
[ 29.657842][ T1269] do_el0_svc+0x50/0x130
[ 29.657850][ T1269] el0_svc+0x24/0x34
[ 29.657858][ T1269] el0_sync_handler+0x180/0x18c
[ 29.657865][ T1269] el0_sync+0x160/0x180
[ 29.657876][ T1269] SMP: stopping secondary CPUs
[ 29.657896][ T1269] Kernel Offset: disabled
[ 29.690695][ T1269] HKRR parameters: disabled
[ 29.691167][ T1269] CPU features: 0x0004,88040002,6a208038
[ 29.691749][ T1269] Memory Limit: none
[ 29.692186][ T1269] ---[ end Kernel panic - not syncing: KASAN: panic_on_warn set ... ]---
Thanks,
Tengda
^ permalink raw reply
* [PATCH v6 2/9] mm/page_owner: add MR_NEVER to enum migrate_reason and use it for last_migrate_reason
From: Ye Liu @ 2026-07-14 1:51 UTC (permalink / raw)
To: Andrew Morton, David Hildenbrand, Steven Rostedt,
Masami Hiramatsu, Vlastimil Babka, Jan Kiszka, Kieran Bingham
Cc: Ye Liu, Zi Yan, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Mathieu Desnoyers, Suren Baghdasaryan, Michal Hocko,
Brendan Jackman, Johannes Weiner, linux-mm, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260714015117.78351-1-ye.liu@linux.dev>
The last_migrate_reason field uses -1 as a sentinel value to mean "no
migration has happened". Replace the four bare -1 occurrences by
adding a proper MR_NEVER member to enum migrate_reason, defining a
corresponding "never_migrated" string in the MIGRATE_REASON trace
macro, and updating the GDB page_owner script to use MR_NEVER instead
of the hardcoded -1 so that lx-dump-page-owner does not incorrectly
report unmigrated pages as migrated.
No functional change.
Signed-off-by: Ye Liu <ye.liu@linux.dev>
Reviewed-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
include/linux/migrate_mode.h | 1 +
include/trace/events/migrate.h | 3 ++-
mm/page_owner.c | 8 ++++----
scripts/gdb/linux/page_owner.py | 4 +++-
4 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/linux/migrate_mode.h b/include/linux/migrate_mode.h
index 265c4328b36a..05102d4d2490 100644
--- a/include/linux/migrate_mode.h
+++ b/include/linux/migrate_mode.h
@@ -25,6 +25,7 @@ enum migrate_reason {
MR_LONGTERM_PIN,
MR_DEMOTION,
MR_DAMON,
+ MR_NEVER, /* page has never been migrated */
MR_TYPES
};
diff --git a/include/trace/events/migrate.h b/include/trace/events/migrate.h
index cd01dd7b3640..11bc0aa14c7e 100644
--- a/include/trace/events/migrate.h
+++ b/include/trace/events/migrate.h
@@ -23,7 +23,8 @@
EM( MR_CONTIG_RANGE, "contig_range") \
EM( MR_LONGTERM_PIN, "longterm_pin") \
EM( MR_DEMOTION, "demotion") \
- EMe(MR_DAMON, "damon")
+ EM( MR_DAMON, "damon") \
+ EMe(MR_NEVER, "never_migrated")
/*
* First define the enums in the above macros to be exported to userspace
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 342549891a8d..c2f43ab860eb 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -339,7 +339,7 @@ noinline void __set_page_owner(struct page *page, unsigned short order,
depot_stack_handle_t handle;
handle = save_stack(gfp_mask);
- __update_page_owner_handle(page, handle, order, gfp_mask, -1,
+ __update_page_owner_handle(page, handle, order, gfp_mask, MR_NEVER,
ts_nsec, current->pid, current->tgid,
current->comm);
inc_stack_record_count(handle, gfp_mask, 1 << order);
@@ -596,7 +596,7 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
if (ret >= count)
goto err;
- if (page_owner->last_migrate_reason != -1) {
+ if (page_owner->last_migrate_reason != MR_NEVER) {
ret += scnprintf(kbuf + ret, count - ret,
"Page has been migrated, last migrate reason: %s\n",
migrate_reason_names[page_owner->last_migrate_reason]);
@@ -667,7 +667,7 @@ void __dump_page_owner(const struct page *page)
stack_depot_print(handle);
}
- if (page_owner->last_migrate_reason != -1)
+ if (page_owner->last_migrate_reason != MR_NEVER)
pr_alert("page has been migrated, last migrate reason: %s\n",
migrate_reason_names[page_owner->last_migrate_reason]);
page_ext_put(page_ext);
@@ -826,7 +826,7 @@ static void init_pages_in_zone(struct zone *zone)
/* Found early allocated page */
__update_page_owner_handle(page, early_handle, 0, 0,
- -1, local_clock(), current->pid,
+ MR_NEVER, local_clock(), current->pid,
current->tgid, current->comm);
count++;
ext_put_continue:
diff --git a/scripts/gdb/linux/page_owner.py b/scripts/gdb/linux/page_owner.py
index 8e713a09cfe7..eeabaeed438b 100644
--- a/scripts/gdb/linux/page_owner.py
+++ b/scripts/gdb/linux/page_owner.py
@@ -34,6 +34,7 @@ class DumpPageOwner(gdb.Command):
max_pfn = None
p_ops = None
migrate_reason_names = None
+ mr_never = None
def __init__(self):
super(DumpPageOwner, self).__init__("lx-dump-page-owner", gdb.COMMAND_SUPPORT)
@@ -65,6 +66,7 @@ class DumpPageOwner(gdb.Command):
self.max_pfn = int(gdb.parse_and_eval("max_pfn"))
self.page_ext_size = int(gdb.parse_and_eval("page_ext_size"))
self.migrate_reason_names = gdb.parse_and_eval('migrate_reason_names')
+ self.mr_never = int(gdb.parse_and_eval('MR_NEVER'))
def page_ext_invalid(self, page_ext):
if page_ext == gdb.Value(0):
@@ -138,7 +140,7 @@ class DumpPageOwner(gdb.Command):
else:
gdb.write('page last free stack trace:\n')
stackdepot.stack_depot_print(page_owner["free_handle"])
- if page_owner['last_migrate_reason'] != -1:
+ if page_owner['last_migrate_reason'] != self.mr_never:
gdb.write('page has been migrated, last migrate reason: %s\n' % self.migrate_reason_names[page_owner['last_migrate_reason']])
def read_page_owner(self):
--
2.43.0
^ permalink raw reply related
* [PATCH v6 3/9] mm: use enum migrate_reason instead of int for migration reason parameters
From: Ye Liu @ 2026-07-14 1:51 UTC (permalink / raw)
To: Muchun Song, Oscar Salvador, Andrew Morton, David Hildenbrand,
Steven Rostedt, Masami Hiramatsu, Vlastimil Babka
Cc: Ye Liu, Zi Yan, Lorenzo Stoakes, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Liam R. Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Mathieu Desnoyers,
Brendan Jackman, Johannes Weiner, linux-mm, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260714015117.78351-1-ye.liu@linux.dev>
Replace all 'int reason' function parameters that carry migrate_reason
values with the proper 'enum migrate_reason' type. This makes the
intent explicit and leverages compiler type checking. The affected
subsystems are:
- page_owner: __folio_set_owner_migrate_reason(),
folio_set_owner_migrate_reason()
- migrate: migrate_pages(), migrate_pages_sync(),
migrate_pages_batch(), migrate_folios_move(),
migrate_hugetlbs(), unmap_and_move_huge_page()
- hugetlb: move_hugetlb_state(), htlb_allow_alloc_fallback()
- trace: mm_migrate_pages and mm_migrate_pages_start events
The 'short last_migrate_reason' struct field and internal helper
parameter in page_owner are intentionally left as 'short' since they
store per-page metadata where size matters.
No functional change.
Signed-off-by: Ye Liu <ye.liu@linux.dev>
Reviewed-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
---
Changes in v6:
- Drop unnecessary 'extern' from __folio_set_owner_migrate_reason()
declaration in page_owner.h.
- Adjust the indentation.
- Link: https://lore.kernel.org/all/20260701061101.344679-4-ye.liu@linux.dev/
include/linux/hugetlb.h | 9 +++++----
include/linux/migrate.h | 8 +++++---
include/linux/page_owner.h | 7 ++++---
include/trace/events/migrate.h | 8 ++++----
mm/hugetlb.c | 3 ++-
mm/migrate.c | 12 ++++++------
mm/page_owner.c | 2 +-
7 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 2abaf99321e9..d6967fc94a49 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -154,7 +154,8 @@ long hugetlb_unreserve_pages(struct inode *inode, long start, long end,
bool folio_isolate_hugetlb(struct folio *folio, struct list_head *list);
int get_hwpoison_hugetlb_folio(struct folio *folio, bool *hugetlb, bool unpoison);
void folio_putback_hugetlb(struct folio *folio);
-void move_hugetlb_state(struct folio *old_folio, struct folio *new_folio, int reason);
+void move_hugetlb_state(struct folio *old_folio, struct folio *new_folio,
+ enum migrate_reason reason);
void hugetlb_fix_reserve_counts(struct inode *inode);
extern struct mutex *hugetlb_fault_mutex_table;
u32 hugetlb_fault_mutex_hash(struct address_space *mapping, pgoff_t idx);
@@ -424,7 +425,7 @@ static inline void folio_putback_hugetlb(struct folio *folio)
}
static inline void move_hugetlb_state(struct folio *old_folio,
- struct folio *new_folio, int reason)
+ struct folio *new_folio, enum migrate_reason reason)
{
}
@@ -956,7 +957,7 @@ static inline gfp_t htlb_modify_alloc_mask(struct hstate *h, gfp_t gfp_mask)
return modified_mask;
}
-static inline bool htlb_allow_alloc_fallback(int reason)
+static inline bool htlb_allow_alloc_fallback(enum migrate_reason reason)
{
bool allowed_fallback = false;
@@ -1238,7 +1239,7 @@ static inline gfp_t htlb_modify_alloc_mask(struct hstate *h, gfp_t gfp_mask)
return 0;
}
-static inline bool htlb_allow_alloc_fallback(int reason)
+static inline bool htlb_allow_alloc_fallback(enum migrate_reason reason)
{
return false;
}
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index d5af2b7f577b..78424b3824c2 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -57,8 +57,9 @@ void putback_movable_pages(struct list_head *l);
int migrate_folio(struct address_space *mapping, struct folio *dst,
struct folio *src, enum migrate_mode mode);
int migrate_pages(struct list_head *l, new_folio_t new, free_folio_t free,
- unsigned long private, enum migrate_mode mode, int reason,
- unsigned int *ret_succeeded);
+ unsigned long private, enum migrate_mode mode,
+ enum migrate_reason reason,
+ unsigned int *ret_succeeded);
struct folio *alloc_migration_target(struct folio *src, unsigned long private);
bool isolate_movable_ops_page(struct page *page, isolate_mode_t mode);
bool isolate_folio_to_list(struct folio *folio, struct list_head *list);
@@ -77,7 +78,8 @@ int set_movable_ops(const struct movable_operations *ops, enum pagetype type);
static inline void putback_movable_pages(struct list_head *l) {}
static inline int migrate_pages(struct list_head *l, new_folio_t new,
free_folio_t free, unsigned long private,
- enum migrate_mode mode, int reason, unsigned int *ret_succeeded)
+ enum migrate_mode mode, enum migrate_reason reason,
+ unsigned int *ret_succeeded)
{ return -ENOSYS; }
static inline struct folio *alloc_migration_target(struct folio *src,
unsigned long private)
diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h
index 3328357f6dba..8188ddc5c412 100644
--- a/include/linux/page_owner.h
+++ b/include/linux/page_owner.h
@@ -3,6 +3,7 @@
#define __LINUX_PAGE_OWNER_H
#include <linux/jump_label.h>
+#include <linux/migrate_mode.h>
#ifdef CONFIG_PAGE_OWNER
extern struct static_key_false page_owner_inited;
@@ -14,7 +15,7 @@ extern void __set_page_owner(struct page *page,
extern void __split_page_owner(struct page *page, int old_order,
int new_order);
extern void __folio_copy_owner(struct folio *newfolio, struct folio *old);
-extern void __folio_set_owner_migrate_reason(struct folio *folio, int reason);
+void __folio_set_owner_migrate_reason(struct folio *folio, enum migrate_reason reason);
extern void __dump_page_owner(const struct page *page);
extern void pagetypeinfo_showmixedcount_print(struct seq_file *m,
pg_data_t *pgdat, struct zone *zone);
@@ -43,7 +44,7 @@ static inline void folio_copy_owner(struct folio *newfolio, struct folio *old)
if (static_branch_unlikely(&page_owner_inited))
__folio_copy_owner(newfolio, old);
}
-static inline void folio_set_owner_migrate_reason(struct folio *folio, int reason)
+static inline void folio_set_owner_migrate_reason(struct folio *folio, enum migrate_reason reason)
{
if (static_branch_unlikely(&page_owner_inited))
__folio_set_owner_migrate_reason(folio, reason);
@@ -68,7 +69,7 @@ static inline void split_page_owner(struct page *page, int old_order,
static inline void folio_copy_owner(struct folio *newfolio, struct folio *folio)
{
}
-static inline void folio_set_owner_migrate_reason(struct folio *folio, int reason)
+static inline void folio_set_owner_migrate_reason(struct folio *folio, enum migrate_reason reason)
{
}
static inline void dump_page_owner(const struct page *page)
diff --git a/include/trace/events/migrate.h b/include/trace/events/migrate.h
index 11bc0aa14c7e..15ee2ef201b5 100644
--- a/include/trace/events/migrate.h
+++ b/include/trace/events/migrate.h
@@ -52,7 +52,7 @@ TRACE_EVENT(mm_migrate_pages,
TP_PROTO(unsigned long succeeded, unsigned long failed,
unsigned long thp_succeeded, unsigned long thp_failed,
unsigned long thp_split, unsigned long large_folio_split,
- enum migrate_mode mode, int reason),
+ enum migrate_mode mode, enum migrate_reason reason),
TP_ARGS(succeeded, failed, thp_succeeded, thp_failed,
thp_split, large_folio_split, mode, reason),
@@ -65,7 +65,7 @@ TRACE_EVENT(mm_migrate_pages,
__field( unsigned long, thp_split)
__field( unsigned long, large_folio_split)
__field( enum migrate_mode, mode)
- __field( int, reason)
+ __field( enum migrate_reason, reason)
),
TP_fast_assign(
@@ -92,13 +92,13 @@ TRACE_EVENT(mm_migrate_pages,
TRACE_EVENT(mm_migrate_pages_start,
- TP_PROTO(enum migrate_mode mode, int reason),
+ TP_PROTO(enum migrate_mode mode, enum migrate_reason reason),
TP_ARGS(mode, reason),
TP_STRUCT__entry(
__field(enum migrate_mode, mode)
- __field(int, reason)
+ __field(enum migrate_reason, reason)
),
TP_fast_assign(
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 571212b80835..17732d1fdc5e 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -7182,7 +7182,8 @@ void folio_putback_hugetlb(struct folio *folio)
folio_put(folio);
}
-void move_hugetlb_state(struct folio *old_folio, struct folio *new_folio, int reason)
+void move_hugetlb_state(struct folio *old_folio, struct folio *new_folio,
+ enum migrate_reason reason)
{
struct hstate *h = folio_hstate(old_folio);
diff --git a/mm/migrate.c b/mm/migrate.c
index d9b23909d716..49e10feeb094 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1469,7 +1469,7 @@ static int migrate_folio_move(free_folio_t put_new_folio, unsigned long private,
static int unmap_and_move_huge_page(new_folio_t get_new_folio,
free_folio_t put_new_folio, unsigned long private,
struct folio *src, int force, enum migrate_mode mode,
- int reason, struct list_head *ret)
+ enum migrate_reason reason, struct list_head *ret)
{
struct folio *dst;
int rc = -EAGAIN;
@@ -1626,7 +1626,7 @@ struct migrate_pages_stats {
*/
static int migrate_hugetlbs(struct list_head *from, new_folio_t get_new_folio,
free_folio_t put_new_folio, unsigned long private,
- enum migrate_mode mode, int reason,
+ enum migrate_mode mode, enum migrate_reason reason,
struct migrate_pages_stats *stats,
struct list_head *ret_folios)
{
@@ -1716,7 +1716,7 @@ static int migrate_hugetlbs(struct list_head *from, new_folio_t get_new_folio,
static void migrate_folios_move(struct list_head *src_folios,
struct list_head *dst_folios,
free_folio_t put_new_folio, unsigned long private,
- enum migrate_mode mode, int reason,
+ enum migrate_mode mode, enum migrate_reason reason,
struct list_head *ret_folios,
struct migrate_pages_stats *stats,
int *retry, int *thp_retry, int *nr_failed,
@@ -1799,7 +1799,7 @@ static void migrate_folios_undo(struct list_head *src_folios,
*/
static int migrate_pages_batch(struct list_head *from,
new_folio_t get_new_folio, free_folio_t put_new_folio,
- unsigned long private, enum migrate_mode mode, int reason,
+ unsigned long private, enum migrate_mode mode, enum migrate_reason reason,
struct list_head *ret_folios, struct list_head *split_folios,
struct migrate_pages_stats *stats, int nr_pass)
{
@@ -2011,7 +2011,7 @@ static int migrate_pages_batch(struct list_head *from,
static int migrate_pages_sync(struct list_head *from, new_folio_t get_new_folio,
free_folio_t put_new_folio, unsigned long private,
- enum migrate_mode mode, int reason,
+ enum migrate_mode mode, enum migrate_reason reason,
struct list_head *ret_folios, struct list_head *split_folios,
struct migrate_pages_stats *stats)
{
@@ -2088,7 +2088,7 @@ static int migrate_pages_sync(struct list_head *from, new_folio_t get_new_folio,
*/
int migrate_pages(struct list_head *from, new_folio_t get_new_folio,
free_folio_t put_new_folio, unsigned long private,
- enum migrate_mode mode, int reason, unsigned int *ret_succeeded)
+ enum migrate_mode mode, enum migrate_reason reason, unsigned int *ret_succeeded)
{
int rc, rc_gather;
int nr_pages;
diff --git a/mm/page_owner.c b/mm/page_owner.c
index c2f43ab860eb..4e352941a6e2 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -345,7 +345,7 @@ noinline void __set_page_owner(struct page *page, unsigned short order,
inc_stack_record_count(handle, gfp_mask, 1 << order);
}
-void __folio_set_owner_migrate_reason(struct folio *folio, int reason)
+void __folio_set_owner_migrate_reason(struct folio *folio, enum migrate_reason reason)
{
struct page_ext *page_ext = page_ext_get(&folio->page);
struct page_owner *page_owner;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] tracing: Add mutex to trace_parser to fix concurrent write races
From: Steven Rostedt @ 2026-07-14 2:13 UTC (permalink / raw)
To: Tengda Wu
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel
In-Reply-To: <ae69656e-fc50-4049-94f9-d460c2e1d5b8@huaweicloud.com>
On Tue, 14 Jul 2026 09:50:55 +0800
Tengda Wu <wutengda@huaweicloud.com> wrote:
> >> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> >> index f93e34dd2328..ef47e5659283 100644
> >> --- a/kernel/trace/ftrace.c
> >> +++ b/kernel/trace/ftrace.c
> >> @@ -5842,6 +5842,8 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
> >> /* iter->hash is a local copy, so we don't need regex_lock */
> >>
> >> parser = &iter->parser;
> >> +
> >> + guard(mutex)(&parser->lock);
> >> read = trace_get_user(parser, ubuf, cnt, ppos);
> >
> > Why are the other users of trace_get_user() not a problem? If
> > anything, trace_get_user() should have a lockdep assert to make sure
> > the lock is held.
> >
> > I think we need to add a lockdep assertion in all the callers that use
> > the parser and we need to make sure it's taken by every user.
> >
> > -- Steve
>
> Other places that use trace_get_user():
>
> ftrace_event_write
> trace_parser_get_init
> trace_get_user
>
> trace_pid_write
> trace_parser_get_init
> trace_get_user
>
> In both of these cases, the parser is allocated via trace_parser_get_init()
> before use and freed immediately after, with no multi-threaded sharing.
>
> However, ftrace_graph_write() and ftrace_regex_write() are different.
> The parser used by these two functions is allocated at open() time and
> retrieved from struct file at write() time. Userspace can have multiple
> threads invoking write() concurrently, e.g.:
>
> r0 = openat(AT_FDCWD, path, O_WRONLY | O_CREAT | O_TRUNC |
> O_NOCTTY | O_NONBLOCK, 0);
> r1 = dup(r0);
> write(r1, data1, len1); /* thread 1 */
> pwrite64(r1, data2, len2, offset); /* thread 2 */
>
> Without proper synchronization in trace_get_user(), the parser state
> becomes undefined.
>
> We have locally reproduced a slab-out-of-bounds issue with syzkaller [1],
> which appears to be caused by this race.
>
> Regarding the fix, adding a lockdep assertion to trace_get_user() does
> not seem necessary. As mentioned above, ftrace_event_write() and
> trace_pid_write() do not involve concurrency, so they do not require
> locking, and thus do not need a lockdep assertion.
So basically you want to add this inherit coupling between a user of
"parser" and the "parser" itself? That is just bad design. You created a
lock within the trace_parser struct to be used by a single instance but not
all instances.
It is either added for all users or this is fixed by something else. Yeah,
for now the other users of trace_get_user() do not need locking, but what
happens the usage changes where it adds concurrency? Then it becomes a bug
again. Right?
If one place needs to hold the parser->lock when calling trace_get_user()
and trace_parser_loaded(), then all places should hold that lock. Or we
just need to redesign it better.
Yes, this fixes a race condition when user space does something stupid (and
this is something that requires admin privileges). But if we are going to
fix it, might as well do it properly.
If the issue is only with ftrace, then make the lock part of ftrace and not
the parser. Don't create a lock in parser that is determined by how one
uses it whether the user needs to take the lock or not.
-- Steve
^ permalink raw reply
* Re: [PATCH v2 2/2] spi: qcom-geni: add GENI SE registers trace event on error paths
From: Praveen Talari @ 2026-07-14 5:21 UTC (permalink / raw)
To: Mark Brown
Cc: Bjorn Andersson, Konrad Dybcio, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, linux-kernel, linux-arm-msm,
linux-trace-kernel, linux-spi, Mukesh Kumar Savaliya,
Konrad Dybcio
In-Reply-To: <72cb70b0-d89d-405b-b2b9-807e028d6fd3@sirena.org.uk>
Hi
On 13-07-2026 16:16, Mark Brown wrote:
> On Sat, Jul 11, 2026 at 08:21:02AM +0530, Praveen Talari wrote:
>> On 11-07-2026 02:56, Mark Brown wrote:
>>> On Sat, Jul 11, 2026 at 12:18:42AM +0530, Praveen Talari wrote:
>>>> +#include <trace/events/qcom_geni_se.h>
>>> Should this be in rivers/soc/qcom/qcom-geni-se.c (and the first patch?)
>>> - that way if another driver starts using them we won't multiply define
>>> the tracepoints.
>> Yes, you are correct but
>> If this header in drivers/soc/qcom/qcom-geni-se.c, how it will be access
>> trace API in other drivers like i2c, spi and uart?
> The header needs to be in the users but the CREATE_ define that you cut
> needs to be in only one place to actually create the tracepoints.
Sure, got it.
>
>>>> @@ -986,10 +997,13 @@ static irqreturn_t geni_spi_isr(int irq, void *data)
>>>> writel(0, se->base + SE_GENI_TX_WATERMARK_REG);
>>>> dev_err(mas->dev, "Premature done. tx_rem = %d bpw%d\n",
>>>> mas->tx_rem_bytes, mas->cur_bits_per_word);
>>>> + trace_geni_se_regs(se);
>>> SE_GENI_TX_WATERMARK_REG is one of the registers in the tracepoint,
>>> perhaps trace before we write to clear it?
>> it will be captured in m_irq register so it is not required to capture.
> Are you sure that won't lead to user confusion?
No since m_irq can capture most of other bits as well.
Thanks,
Praveen talari
^ permalink raw reply
* Re: [PATCH v2 1/4] mm/migrate: do not migrate folios mapped into VM_LOCKED VMAs under compaction
From: Wandun @ 2026-07-14 6:28 UTC (permalink / raw)
To: Zi Yan, Lorenzo Stoakes
Cc: 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: <6A08B6C8-E94C-4690-A29E-B9A0DAFE19EF@nvidia.com>
On 7/10/26 00:10, Zi Yan wrote:
> 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.
Hi Zi Yan,
Thanks for the suggestion, I really like this approach.
Adding a TTU flag and the corresponding check in the rmap logic does
make the code more complex. With the addition of STOP logic, the
entire patch becomes very clear and readable.
Hi, Lorenzo, would you be happy with Yan's invalid_vma STOP extension?
I'm inclined toward the this since it removes the new flag and reads
more clearly.
Best regards,
Wandun
>
> This approach will not require a new TTU flag, although it requires more
> code changes.
>
> Best Regards,
> Yan, Zi
^ permalink raw reply
* [PATCH] rv: Simplify task monitor slot management
From: liqiang @ 2026-07-14 8:33 UTC (permalink / raw)
To: linux-trace-kernel
Cc: rostedt, gmonaco, mhiramat, mathieu.desnoyers, linux-kernel,
liqiang
The slot array already tracks allocation and task_monitor_count
duplicates that state. On an invalid second release, the old code
warns but still decrements the counter, corrupting later allocations.
Use the slot array as the sole source of truth. Return after warning
about an unused slot, and return -EBUSY when no slot is free.
Signed-off-by: liqiang <liqiang01@kylinos.cn>
---
kernel/trace/rv/rv.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
index ee4e68102f17..187d87d5991c 100644
--- a/kernel/trace/rv/rv.c
+++ b/kernel/trace/rv/rv.c
@@ -164,7 +164,6 @@ struct dentry *get_monitors_root(void)
*/
LIST_HEAD(rv_monitors_list);
-static int task_monitor_count;
static bool task_monitor_slots[CONFIG_RV_PER_TASK_MONITORS];
int rv_get_task_monitor_slot(void)
@@ -173,21 +172,14 @@ int rv_get_task_monitor_slot(void)
lockdep_assert_held(&rv_interface_lock);
- if (task_monitor_count == CONFIG_RV_PER_TASK_MONITORS)
- return -EBUSY;
-
- task_monitor_count++;
-
for (i = 0; i < CONFIG_RV_PER_TASK_MONITORS; i++) {
- if (task_monitor_slots[i] == false) {
+ if (!task_monitor_slots[i]) {
task_monitor_slots[i] = true;
return i;
}
}
- WARN_ONCE(1, "RV task_monitor_count and slots are out of sync\n");
-
- return -EINVAL;
+ return -EBUSY;
}
void rv_put_task_monitor_slot(int slot)
@@ -199,10 +191,10 @@ void rv_put_task_monitor_slot(int slot)
return;
}
- WARN_ONCE(!task_monitor_slots[slot], "RV releasing unused task_monitor_slots: %d\n",
- slot);
+ if (WARN_ONCE(!task_monitor_slots[slot],
+ "RV releasing unused task monitor slot: %d\n", slot))
+ return;
- task_monitor_count--;
task_monitor_slots[slot] = false;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] tracing: Add mutex to trace_parser to fix concurrent write races
From: Tengda Wu @ 2026-07-14 10:16 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel
In-Reply-To: <20260713221318.63f741ad@gandalf.local.home>
On 2026/7/14 10:13, Steven Rostedt wrote:
> On Tue, 14 Jul 2026 09:50:55 +0800
> Tengda Wu <wutengda@huaweicloud.com> wrote:
>
>>>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>>>> index f93e34dd2328..ef47e5659283 100644
>>>> --- a/kernel/trace/ftrace.c
>>>> +++ b/kernel/trace/ftrace.c
>>>> @@ -5842,6 +5842,8 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
>>>> /* iter->hash is a local copy, so we don't need regex_lock */
>>>>
>>>> parser = &iter->parser;
>>>> +
>>>> + guard(mutex)(&parser->lock);
>>>> read = trace_get_user(parser, ubuf, cnt, ppos);
>>>
>>> Why are the other users of trace_get_user() not a problem? If
>>> anything, trace_get_user() should have a lockdep assert to make sure
>>> the lock is held.
>>>
>>> I think we need to add a lockdep assertion in all the callers that use
>>> the parser and we need to make sure it's taken by every user.
>>>
>>> -- Steve
>>
>> Other places that use trace_get_user():
>>
>> ftrace_event_write
>> trace_parser_get_init
>> trace_get_user
>>
>> trace_pid_write
>> trace_parser_get_init
>> trace_get_user
>>
>> In both of these cases, the parser is allocated via trace_parser_get_init()
>> before use and freed immediately after, with no multi-threaded sharing.
>>
>> However, ftrace_graph_write() and ftrace_regex_write() are different.
>> The parser used by these two functions is allocated at open() time and
>> retrieved from struct file at write() time. Userspace can have multiple
>> threads invoking write() concurrently, e.g.:
>>
>> r0 = openat(AT_FDCWD, path, O_WRONLY | O_CREAT | O_TRUNC |
>> O_NOCTTY | O_NONBLOCK, 0);
>> r1 = dup(r0);
>> write(r1, data1, len1); /* thread 1 */
>> pwrite64(r1, data2, len2, offset); /* thread 2 */
>>
>> Without proper synchronization in trace_get_user(), the parser state
>> becomes undefined.
>>
>> We have locally reproduced a slab-out-of-bounds issue with syzkaller [1],
>> which appears to be caused by this race.
>>
>> Regarding the fix, adding a lockdep assertion to trace_get_user() does
>> not seem necessary. As mentioned above, ftrace_event_write() and
>> trace_pid_write() do not involve concurrency, so they do not require
>> locking, and thus do not need a lockdep assertion.
>
> So basically you want to add this inherit coupling between a user of
> "parser" and the "parser" itself? That is just bad design. You created a
> lock within the trace_parser struct to be used by a single instance but not
> all instances.
>
You're right. That coupling is indeed bad.
> It is either added for all users or this is fixed by something else. Yeah,
> for now the other users of trace_get_user() do not need locking, but what
> happens the usage changes where it adds concurrency? Then it becomes a bug
> again. Right?
>
> If one place needs to hold the parser->lock when calling trace_get_user()
> and trace_parser_loaded(), then all places should hold that lock. Or we
> just need to redesign it better.
>
> Yes, this fixes a race condition when user space does something stupid (and
> this is something that requires admin privileges). But if we are going to
> fix it, might as well do it properly.
>
> If the issue is only with ftrace, then make the lock part of ftrace and not
> the parser. Don't create a lock in parser that is determined by how one
> uses it whether the user needs to take the lock or not.
>
Yes, the issue at hand is currently limited to ftrace internals,
specifically in ftrace_graph_write() and ftrace_regex_write(). I did
consider moving the lock out of the parser and confining it within
ftrace.c, but the approaches I came up with so far all seem less than
ideal. Let me outline them below.
_Approach 1_: Add a global lock at the ftrace.c file level, and hold it
in the relevant write entry points for mutual exclusion.
The obvious downside here is that both of the aforementioned functions are
called by multiple tracefs file write hooks. A single global mutex would
cause unnecessary blocking between writes to different files. On the other
hand, introducing multiple global mutexes (one per file) would scatter the
lock definitions and make maintenance harder.
ftrace_graph_write() is used by:
- set_graph_function
- set_graph_notrace
ftrace_regex_write() is used by:
- set_ftrace_filter
- set_ftrace_notrace
_Approach 2_: Move the lock into structures such as ftrace_graph_data
or ftrace_iterator, both of which already embed a trace_parser.
While this does fix the problem, it feels somewhat redundant. We would be
defining essentially the same lock in multiple places just to serialize
access to the parser. Right now that would mean two definitions, but in the
future, depending on how many parser users need concurrency control, it
could grow further.
_Approach 3_: As you suggested earlier, build on v1 but add proper lockdep
assertions to enforce that the parser lock is held by all callers,
regardless of how the parser is used.
Approach 3 seems cleaner and more straightforward.
Do you happen to have a better suggestion in mind? I'm open to other
approaches if we can keep the design clean and maintainable.
Thanks,
Tengda
^ permalink raw reply
* Re: [PATCH v2 13/33] mm/vma: introduce and use vmg_pages(), vmg_[start, end]_pgoff()
From: Lorenzo Stoakes (ARM) @ 2026-07-14 10:17 UTC (permalink / raw)
To: Vlastimil Babka (SUSE)
Cc: Andrew Morton, David Hildenbrand, Liam R. Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Rik van Riel, Harry Yoo,
Jann Horn, Lance Yang, Pedro Falcato, Russell King, Dinh Nguyen,
Simon Schuster, James E.J. Bottomley, Helge Deller,
Alexander Viro, Christian Brauner, Jan Kara, Dan Williams,
Matthew Wilcox, Muchun Song, Oscar Salvador, Masami Hiramatsu,
Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif,
linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <508301c5-5460-40e8-bc34-6410be5be40a@kernel.org>
On Mon, Jul 13, 2026 at 08:12:24PM +0200, Vlastimil Babka (SUSE) wrote:
> On 7/10/26 22:16, Lorenzo Stoakes wrote:
> > In the VMA logic we often need to determine the number of pages in the
> > specified merge range, as well as the start and end page offsets of that
> > range.
> >
> > Introduce and use helpers for these purposes.
> >
> > No functional change intended.
> >
> > Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> > Reviewed-by: Gregory Price <gourry@gourry.net>
> > Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>
> Uh, the PHYS_PFN() usage here was quite an abuse. Good riddance.
Yeah and mine :)) but absolutely agree it was wrong and glad to fix my own
mistake in using that ;)
> Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Thanks!
>
> > ---
> > mm/vma.c | 11 ++++-------
> > mm/vma.h | 17 +++++++++++++++++
> > 2 files changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/mm/vma.c b/mm/vma.c
> > index 35ba3475128f..995de8198fbb 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -197,11 +197,9 @@ static void init_multi_vma_prep(struct vma_prepare *vp,
> > */
> > static bool can_vma_merge_before(struct vma_merge_struct *vmg)
> > {
> > - pgoff_t pglen = PHYS_PFN(vmg->end - vmg->start);
> > -
> > if (is_mergeable_vma(vmg, /* merge_next = */ true) &&
> > is_mergeable_anon_vma(vmg, /* merge_next = */ true)) {
> > - if (vmg->next->vm_pgoff == vmg->pgoff + pglen)
> > + if (vmg_end_pgoff(vmg) == vma_start_pgoff(vmg->next))
> > return true;
> > }
> >
> > @@ -221,7 +219,7 @@ static bool can_vma_merge_after(struct vma_merge_struct *vmg)
> > {
> > if (is_mergeable_vma(vmg, /* merge_next = */ false) &&
> > is_mergeable_anon_vma(vmg, /* merge_next = */ false)) {
> > - if (vmg->prev->vm_pgoff + vma_pages(vmg->prev) == vmg->pgoff)
> > + if (vma_end_pgoff(vmg->prev) == vmg_start_pgoff(vmg))
> > return true;
> > }
> > return false;
> > @@ -759,7 +757,7 @@ static int commit_merge(struct vma_merge_struct *vmg)
> > */
> > vma_adjust_trans_huge(vma, vmg->start, vmg->end,
> > vmg->__adjust_middle_start ? vmg->middle : NULL);
> > - vma_set_range(vma, vmg->start, vmg->end, vmg->pgoff);
> > + vma_set_range(vma, vmg->start, vmg->end, vmg_start_pgoff(vmg));
> > vmg_adjust_set_range(vmg);
> > vma_iter_store_overwrite(vmg->vmi, vmg->target);
> >
> > @@ -962,8 +960,7 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
> > * middle next
> > * shrink/delete extend
> > */
> > -
> > - pgoff_t pglen = PHYS_PFN(vmg->end - vmg->start);
> > + const pgoff_t pglen = vmg_pages(vmg);
> >
> > VM_WARN_ON_VMG(!merge_right, vmg);
> > /* If we are offset into a VMA, then prev must be middle. */
> > diff --git a/mm/vma.h b/mm/vma.h
> > index 8e4b61a7304c..527716c8739d 100644
> > --- a/mm/vma.h
> > +++ b/mm/vma.h
> > @@ -230,6 +230,23 @@ static inline bool vmg_nomem(struct vma_merge_struct *vmg)
> > return vmg->state == VMA_MERGE_ERROR_NOMEM;
> > }
> >
> > +static inline pgoff_t vmg_start_pgoff(const struct vma_merge_struct *vmg)
> > +{
> > + return vmg->pgoff;
> > +}
> > +
> > +static inline pgoff_t vmg_pages(const struct vma_merge_struct *vmg)
> > +{
> > + const unsigned long size = vmg->end - vmg->start;
> > +
> > + return size >> PAGE_SHIFT;
> > +}
> > +
> > +static inline pgoff_t vmg_end_pgoff(const struct vma_merge_struct *vmg)
> > +{
> > + return vmg_start_pgoff(vmg) + vmg_pages(vmg);
> > +}
> > +
> > /* Assumes addr >= vma->vm_start. */
> > static inline pgoff_t vma_pgoff_offset(struct vm_area_struct *vma,
> > unsigned long addr)
> >
>
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v2 2/2] spi: qcom-geni: add GENI SE registers trace event on error paths
From: Mark Brown @ 2026-07-14 10:24 UTC (permalink / raw)
To: Praveen Talari
Cc: Bjorn Andersson, Konrad Dybcio, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, linux-kernel, linux-arm-msm,
linux-trace-kernel, linux-spi, Mukesh Kumar Savaliya,
Konrad Dybcio
In-Reply-To: <788b1ebb-db13-4f6f-9e92-46802d7027f6@oss.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 620 bytes --]
On Tue, Jul 14, 2026 at 10:51:42AM +0530, Praveen Talari wrote:
> On 13-07-2026 16:16, Mark Brown wrote:
> > > > SE_GENI_TX_WATERMARK_REG is one of the registers in the tracepoint,
> > > > perhaps trace before we write to clear it?
> > > it will be captured in m_irq register so it is not required to capture.
> > Are you sure that won't lead to user confusion?
> No since m_irq can capture most of other bits as well.
The point isn't that the data isn't somewhere else, the point is that
the user might look at the watermark value and not remember that in this
one specific case they need to check something else.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] tracing: Add mutex to trace_parser to fix concurrent write races
From: Steven Rostedt @ 2026-07-14 11:24 UTC (permalink / raw)
To: Tengda Wu
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel
In-Reply-To: <059bee5e-6852-4afb-a993-dd5ddcd86d3e@huaweicloud.com>
On Tue, 14 Jul 2026 18:16:26 +0800
Tengda Wu <wutengda@huaweicloud.com> wrote:
> _Approach 3_: As you suggested earlier, build on v1 but add proper lockdep
> assertions to enforce that the parser lock is held by all callers,
> regardless of how the parser is used.
>
> Approach 3 seems cleaner and more straightforward.
I thought about having a global lock as writes to tracefs are very much a
slow path, but I think Approach 3 is probably the most "correct" fix.
Yeah, let's go with that.
Thanks,
-- Steve
^ permalink raw reply
* Re: [PATCH] rv: Simplify task monitor slot management
From: Gabriele Monaco @ 2026-07-14 11:27 UTC (permalink / raw)
To: liqiang, linux-trace-kernel
Cc: rostedt, mhiramat, mathieu.desnoyers, linux-kernel
In-Reply-To: <20260714083315.1371676-1-liqiang01@kylinos.cn>
On Tue, 2026-07-14 at 16:33 +0800, liqiang wrote:
> The slot array already tracks allocation and task_monitor_count
> duplicates that state. On an invalid second release, the old code
> warns but still decrements the counter, corrupting later allocations.
>
> Use the slot array as the sole source of truth. Return after warning
> about an unused slot, and return -EBUSY when no slot is free.
>
> Signed-off-by: liqiang <liqiang01@kylinos.cn>
Thanks for the patch! It looks good to me:
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
However, your signoff should include a "known identity" [1]. For most people
this is an official name and surname (Li Qiang ?).
You could probably just reply with the new signoff, but it's probably better you
send a V2 with it updated. Feel free to also include my review tag.
I suggest you configure git accordingly e.g.:
git config set user.name "Name Surname"
git commit --amend --reset-author --signoff
# will open an editor from there remove the old signoff and add the review tag
Thanks,
Gabriele
[1] -
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
> ---
> kernel/trace/rv/rv.c | 18 +++++-------------
> 1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
> index ee4e68102f17..187d87d5991c 100644
> --- a/kernel/trace/rv/rv.c
> +++ b/kernel/trace/rv/rv.c
> @@ -164,7 +164,6 @@ struct dentry *get_monitors_root(void)
> */
> LIST_HEAD(rv_monitors_list);
>
> -static int task_monitor_count;
> static bool task_monitor_slots[CONFIG_RV_PER_TASK_MONITORS];
>
> int rv_get_task_monitor_slot(void)
> @@ -173,21 +172,14 @@ int rv_get_task_monitor_slot(void)
>
> lockdep_assert_held(&rv_interface_lock);
>
> - if (task_monitor_count == CONFIG_RV_PER_TASK_MONITORS)
> - return -EBUSY;
> -
> - task_monitor_count++;
> -
> for (i = 0; i < CONFIG_RV_PER_TASK_MONITORS; i++) {
> - if (task_monitor_slots[i] == false) {
> + if (!task_monitor_slots[i]) {
> task_monitor_slots[i] = true;
> return i;
> }
> }
>
> - WARN_ONCE(1, "RV task_monitor_count and slots are out of sync\n");
> -
> - return -EINVAL;
> + return -EBUSY;
> }
>
> void rv_put_task_monitor_slot(int slot)
> @@ -199,10 +191,10 @@ void rv_put_task_monitor_slot(int slot)
> return;
> }
>
> - WARN_ONCE(!task_monitor_slots[slot], "RV releasing unused
> task_monitor_slots: %d\n",
> - slot);
> + if (WARN_ONCE(!task_monitor_slots[slot],
> + "RV releasing unused task monitor slot: %d\n", slot))
> + return;
>
> - task_monitor_count--;
> task_monitor_slots[slot] = false;
> }
>
^ 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