* 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
* Re: [PATCH] tracing: Add mutex to trace_parser to fix concurrent write races
From: Tengda Wu @ 2026-07-14 11:57 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel
In-Reply-To: <20260714072440.4ff92b33@gandalf.local.home>
On 2026/7/14 19:24, Steven Rostedt wrote:
> 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
Thanks, Steve. I'll spin a v2 with the lockdep assertions added.
Tengda
^ permalink raw reply
* Re: [PATCH v6 2/9] mm/page_owner: add MR_NEVER to enum migrate_reason and use it for last_migrate_reason
From: David Hildenbrand (Arm) @ 2026-07-14 13:05 UTC (permalink / raw)
To: Ye Liu, Andrew Morton, Steven Rostedt, Masami Hiramatsu,
Vlastimil Babka, Jan Kiszka, Kieran Bingham
Cc: 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-3-ye.liu@linux.dev>
On 7/14/26 03:51, Ye Liu wrote:
> 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 */
Shouldn't this be more like
"MR_NONE" ?
As "never" is not really a "reason" ?
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v6 3/9] mm: use enum migrate_reason instead of int for migration reason parameters
From: David Hildenbrand (Arm) @ 2026-07-14 13:07 UTC (permalink / raw)
To: Ye Liu, Muchun Song, Oscar Salvador, Andrew Morton,
Steven Rostedt, Masami Hiramatsu, Vlastimil Babka
Cc: 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-4-ye.liu@linux.dev>
On 7/14/26 03:51, Ye Liu wrote:
> 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>
> ---
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v6 2/9] mm/page_owner: add MR_NEVER to enum migrate_reason and use it for last_migrate_reason
From: Vlastimil Babka (SUSE) @ 2026-07-14 13:09 UTC (permalink / raw)
To: David Hildenbrand (Arm), Ye Liu, Andrew Morton, Steven Rostedt,
Masami Hiramatsu, Jan Kiszka, Kieran Bingham
Cc: 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: <9e0db1c2-06ca-4dba-8fb1-552fab91cb02@kernel.org>
On 7/14/26 15:05, David Hildenbrand (Arm) wrote:
> On 7/14/26 03:51, Ye Liu wrote:
>> 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 */
>
> Shouldn't this be more like
>
> "MR_NONE" ?
>
> As "never" is not really a "reason" ?
It's not used ever as a reason that would be actually passed to migration.
So I think the name is more descriptive this way.
^ permalink raw reply
* Re: [PATCH v6 2/9] mm/page_owner: add MR_NEVER to enum migrate_reason and use it for last_migrate_reason
From: David Hildenbrand (Arm) @ 2026-07-14 13:22 UTC (permalink / raw)
To: Vlastimil Babka (SUSE), Ye Liu, Andrew Morton, Steven Rostedt,
Masami Hiramatsu, Jan Kiszka, Kieran Bingham
Cc: 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: <037d58c6-de55-4b69-bb71-905e03985657@kernel.org>
On 7/14/26 15:09, Vlastimil Babka (SUSE) wrote:
> On 7/14/26 15:05, David Hildenbrand (Arm) wrote:
>> On 7/14/26 03:51, Ye Liu wrote:
>>> 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 */
>>
>> Shouldn't this be more like
>>
>> "MR_NONE" ?
>>
>> As "never" is not really a "reason" ?
>
> It's not used ever as a reason that would be actually passed to migration.
Right, it's a placeholder for "there is no migrate reason because it is unset"
> So I think the name is more descriptive this way.
Not sure I agree. The usual translation of -1 -> unset is NONE or UNSET.
At least I was confused by "NEVER".
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v6 2/9] mm/page_owner: add MR_NEVER to enum migrate_reason and use it for last_migrate_reason
From: Vlastimil Babka (SUSE) @ 2026-07-14 13:49 UTC (permalink / raw)
To: David Hildenbrand (Arm), Ye Liu, Andrew Morton, Steven Rostedt,
Masami Hiramatsu, Jan Kiszka, Kieran Bingham
Cc: 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: <9e9345c8-30a4-4f78-ba55-d7fa073b82d5@kernel.org>
On 7/14/26 15:22, David Hildenbrand (Arm) wrote:
> On 7/14/26 15:09, Vlastimil Babka (SUSE) wrote:
>> On 7/14/26 15:05, David Hildenbrand (Arm) wrote:
>>> On 7/14/26 03:51, Ye Liu wrote:
>>>> 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 */
>>>
>>> Shouldn't this be more like
>>>
>>> "MR_NONE" ?
>>>
>>> As "never" is not really a "reason" ?
>>
>> It's not used ever as a reason that would be actually passed to migration.
>
> Right, it's a placeholder for "there is no migrate reason because it is unset"
>
>> So I think the name is more descriptive this way.
>
> Not sure I agree. The usual translation of -1 -> unset is NONE or UNSET.
(note it's no longer -1 after the patch.)
> At least I was confused by "NEVER".
I won't bikeshed this, so whatever.
Just to avoid another extra respin, please also say how the following line
should change, as if we only rename MR_NEVER to MR_NONE, it will be:
+ EMe(MR_NONE, "never_migrated")
Is that ok? Note it should never be actually printed anywhere per the
current code.
^ permalink raw reply
* Re: [PATCH] bpf/btf: Move tracing BTF APIs to the BTF library
From: Steven Rostedt @ 2026-07-14 14:34 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Martin KaFai Lau, Alexei Starovoitov, linux-trace-kernel, bpf,
linux-kernel
In-Reply-To: <169694605862.516358.5321950027838863987.stgit@devnote2>
Whatever happened to this patch?
-- Steve
On Tue, 10 Oct 2023 22:54:19 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Move the BTF APIs used in tracing to the BTF library code for sharing it
> with others.
> Previously, to avoid complex dependency in a series I made it on the
> tracing tree, but now it is a good time to move it to BPF tree because
> these functions are pure BTF functions.
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> include/linux/btf.h | 24 +++++++++
> kernel/bpf/btf.c | 115 +++++++++++++++++++++++++++++++++++++++++
> kernel/trace/Makefile | 1
> kernel/trace/trace_btf.c | 122 --------------------------------------------
> kernel/trace/trace_btf.h | 11 ----
> kernel/trace/trace_probe.c | 2 -
> 6 files changed, 140 insertions(+), 135 deletions(-)
> delete mode 100644 kernel/trace/trace_btf.c
> delete mode 100644 kernel/trace/trace_btf.h
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 928113a80a95..8372d93ea402 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -507,6 +507,14 @@ btf_get_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf,
> int get_kern_ctx_btf_id(struct bpf_verifier_log *log, enum bpf_prog_type prog_type);
> bool btf_types_are_same(const struct btf *btf1, u32 id1,
> const struct btf *btf2, u32 id2);
> +const struct btf_type *btf_find_func_proto(const char *func_name,
> + struct btf **btf_p);
> +const struct btf_param *btf_get_func_param(const struct btf_type *func_proto,
> + s32 *nr);
> +const struct btf_member *btf_find_struct_member(struct btf *btf,
> + const struct btf_type *type,
> + const char *member_name,
> + u32 *anon_offset);
> #else
> static inline const struct btf_type *btf_type_by_id(const struct btf *btf,
> u32 type_id)
> @@ -559,6 +567,22 @@ static inline bool btf_types_are_same(const struct btf *btf1, u32 id1,
> {
> return false;
> }
> +static inline const struct btf_type *btf_find_func_proto(const char *func_name,
> + struct btf **btf_p)
> +{
> + return NULL;
> +}
> +static inline const struct btf_param *
> +btf_get_func_param(const struct btf_type *func_proto, s32 *nr)
> +{
> + return NULL;
> +}
> +static inline const struct btf_member *
> +btf_find_struct_member(struct btf *btf, const struct btf_type *type,
> + const char *member_name, u32 *anon_offset)
> +{
> + return NULL;
> +}
> #endif
>
> static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type *t)
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 8090d7fb11ef..e5cbf3b31b78 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -912,6 +912,121 @@ static const struct btf_type *btf_type_skip_qualifiers(const struct btf *btf,
> return t;
> }
>
> +/*
> + * Find a function proto type by name, and return the btf_type with its btf
> + * in *@btf_p. Return NULL if not found.
> + * Note that caller has to call btf_put(*@btf_p) after using the btf_type.
> + */
> +const struct btf_type *btf_find_func_proto(const char *func_name, struct btf **btf_p)
> +{
> + const struct btf_type *t;
> + s32 id;
> +
> + id = bpf_find_btf_id(func_name, BTF_KIND_FUNC, btf_p);
> + if (id < 0)
> + return NULL;
> +
> + /* Get BTF_KIND_FUNC type */
> + t = btf_type_by_id(*btf_p, id);
> + if (!t || !btf_type_is_func(t))
> + goto err;
> +
> + /* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
> + t = btf_type_by_id(*btf_p, t->type);
> + if (!t || !btf_type_is_func_proto(t))
> + goto err;
> +
> + return t;
> +err:
> + btf_put(*btf_p);
> + return NULL;
> +}
> +
> +/*
> + * Get function parameter with the number of parameters.
> + * This can return NULL if the function has no parameters.
> + * It can return -EINVAL if the @func_proto is not a function proto type.
> + */
> +const struct btf_param *btf_get_func_param(const struct btf_type *func_proto, s32 *nr)
> +{
> + if (!btf_type_is_func_proto(func_proto))
> + return ERR_PTR(-EINVAL);
> +
> + *nr = btf_type_vlen(func_proto);
> + if (*nr > 0)
> + return (const struct btf_param *)(func_proto + 1);
> + else
> + return NULL;
> +}
> +
> +#define BTF_ANON_STACK_MAX 16
> +
> +struct btf_anon_stack {
> + u32 tid;
> + u32 offset;
> +};
> +
> +/*
> + * Find a member of data structure/union by name and return it.
> + * Return NULL if not found, or -EINVAL if parameter is invalid.
> + * If the member is an member of anonymous union/structure, the offset
> + * of that anonymous union/structure is stored into @anon_offset. Caller
> + * can calculate the correct offset from the root data structure by
> + * adding anon_offset to the member's offset.
> + */
> +const struct btf_member *btf_find_struct_member(struct btf *btf,
> + const struct btf_type *type,
> + const char *member_name,
> + u32 *anon_offset)
> +{
> + struct btf_anon_stack *anon_stack;
> + const struct btf_member *member;
> + u32 tid, cur_offset = 0;
> + const char *name;
> + int i, top = 0;
> +
> + anon_stack = kcalloc(BTF_ANON_STACK_MAX, sizeof(*anon_stack), GFP_KERNEL);
> + if (!anon_stack)
> + return ERR_PTR(-ENOMEM);
> +
> +retry:
> + if (!btf_type_is_struct(type)) {
> + member = ERR_PTR(-EINVAL);
> + goto out;
> + }
> +
> + for_each_member(i, type, member) {
> + if (!member->name_off) {
> + /* Anonymous union/struct: push it for later use */
> + type = btf_type_skip_modifiers(btf, member->type, &tid);
> + if (type && top < BTF_ANON_STACK_MAX) {
> + anon_stack[top].tid = tid;
> + anon_stack[top++].offset =
> + cur_offset + member->offset;
> + }
> + } else {
> + name = btf_name_by_offset(btf, member->name_off);
> + if (name && !strcmp(member_name, name)) {
> + if (anon_offset)
> + *anon_offset = cur_offset;
> + goto out;
> + }
> + }
> + }
> + if (top > 0) {
> + /* Pop from the anonymous stack and retry */
> + tid = anon_stack[--top].tid;
> + cur_offset = anon_stack[top].offset;
> + type = btf_type_by_id(btf, tid);
> + goto retry;
> + }
> + member = NULL;
> +
> +out:
> + kfree(anon_stack);
> + return member;
> +}
> +
> #define BTF_SHOW_MAX_ITER 10
>
> #define BTF_KIND_BIT(kind) (1ULL << kind)
> diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> index 057cd975d014..64b61f67a403 100644
> --- a/kernel/trace/Makefile
> +++ b/kernel/trace/Makefile
> @@ -99,7 +99,6 @@ obj-$(CONFIG_KGDB_KDB) += trace_kdb.o
> endif
> obj-$(CONFIG_DYNAMIC_EVENTS) += trace_dynevent.o
> obj-$(CONFIG_PROBE_EVENTS) += trace_probe.o
> -obj-$(CONFIG_PROBE_EVENTS_BTF_ARGS) += trace_btf.o
> obj-$(CONFIG_UPROBE_EVENTS) += trace_uprobe.o
> obj-$(CONFIG_BOOTTIME_TRACING) += trace_boot.o
> obj-$(CONFIG_FTRACE_RECORD_RECURSION) += trace_recursion_record.o
> diff --git a/kernel/trace/trace_btf.c b/kernel/trace/trace_btf.c
> deleted file mode 100644
> index ca224d53bfdc..000000000000
> --- a/kernel/trace/trace_btf.c
> +++ /dev/null
> @@ -1,122 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/btf.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -
> -#include "trace_btf.h"
> -
> -/*
> - * Find a function proto type by name, and return the btf_type with its btf
> - * in *@btf_p. Return NULL if not found.
> - * Note that caller has to call btf_put(*@btf_p) after using the btf_type.
> - */
> -const struct btf_type *btf_find_func_proto(const char *func_name, struct btf **btf_p)
> -{
> - const struct btf_type *t;
> - s32 id;
> -
> - id = bpf_find_btf_id(func_name, BTF_KIND_FUNC, btf_p);
> - if (id < 0)
> - return NULL;
> -
> - /* Get BTF_KIND_FUNC type */
> - t = btf_type_by_id(*btf_p, id);
> - if (!t || !btf_type_is_func(t))
> - goto err;
> -
> - /* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
> - t = btf_type_by_id(*btf_p, t->type);
> - if (!t || !btf_type_is_func_proto(t))
> - goto err;
> -
> - return t;
> -err:
> - btf_put(*btf_p);
> - return NULL;
> -}
> -
> -/*
> - * Get function parameter with the number of parameters.
> - * This can return NULL if the function has no parameters.
> - * It can return -EINVAL if the @func_proto is not a function proto type.
> - */
> -const struct btf_param *btf_get_func_param(const struct btf_type *func_proto, s32 *nr)
> -{
> - if (!btf_type_is_func_proto(func_proto))
> - return ERR_PTR(-EINVAL);
> -
> - *nr = btf_type_vlen(func_proto);
> - if (*nr > 0)
> - return (const struct btf_param *)(func_proto + 1);
> - else
> - return NULL;
> -}
> -
> -#define BTF_ANON_STACK_MAX 16
> -
> -struct btf_anon_stack {
> - u32 tid;
> - u32 offset;
> -};
> -
> -/*
> - * Find a member of data structure/union by name and return it.
> - * Return NULL if not found, or -EINVAL if parameter is invalid.
> - * If the member is an member of anonymous union/structure, the offset
> - * of that anonymous union/structure is stored into @anon_offset. Caller
> - * can calculate the correct offset from the root data structure by
> - * adding anon_offset to the member's offset.
> - */
> -const struct btf_member *btf_find_struct_member(struct btf *btf,
> - const struct btf_type *type,
> - const char *member_name,
> - u32 *anon_offset)
> -{
> - struct btf_anon_stack *anon_stack;
> - const struct btf_member *member;
> - u32 tid, cur_offset = 0;
> - const char *name;
> - int i, top = 0;
> -
> - anon_stack = kcalloc(BTF_ANON_STACK_MAX, sizeof(*anon_stack), GFP_KERNEL);
> - if (!anon_stack)
> - return ERR_PTR(-ENOMEM);
> -
> -retry:
> - if (!btf_type_is_struct(type)) {
> - member = ERR_PTR(-EINVAL);
> - goto out;
> - }
> -
> - for_each_member(i, type, member) {
> - if (!member->name_off) {
> - /* Anonymous union/struct: push it for later use */
> - type = btf_type_skip_modifiers(btf, member->type, &tid);
> - if (type && top < BTF_ANON_STACK_MAX) {
> - anon_stack[top].tid = tid;
> - anon_stack[top++].offset =
> - cur_offset + member->offset;
> - }
> - } else {
> - name = btf_name_by_offset(btf, member->name_off);
> - if (name && !strcmp(member_name, name)) {
> - if (anon_offset)
> - *anon_offset = cur_offset;
> - goto out;
> - }
> - }
> - }
> - if (top > 0) {
> - /* Pop from the anonymous stack and retry */
> - tid = anon_stack[--top].tid;
> - cur_offset = anon_stack[top].offset;
> - type = btf_type_by_id(btf, tid);
> - goto retry;
> - }
> - member = NULL;
> -
> -out:
> - kfree(anon_stack);
> - return member;
> -}
> -
> diff --git a/kernel/trace/trace_btf.h b/kernel/trace/trace_btf.h
> deleted file mode 100644
> index 4bc44bc261e6..000000000000
> --- a/kernel/trace/trace_btf.h
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#include <linux/btf.h>
> -
> -const struct btf_type *btf_find_func_proto(const char *func_name,
> - struct btf **btf_p);
> -const struct btf_param *btf_get_func_param(const struct btf_type *func_proto,
> - s32 *nr);
> -const struct btf_member *btf_find_struct_member(struct btf *btf,
> - const struct btf_type *type,
> - const char *member_name,
> - u32 *anon_offset);
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 4dc74d73fc1d..b33c424b8ee0 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -12,7 +12,7 @@
> #define pr_fmt(fmt) "trace_probe: " fmt
>
> #include <linux/bpf.h>
> -#include "trace_btf.h"
> +#include <linux/btf.h>
>
> #include "trace_probe.h"
>
^ permalink raw reply
* Re: [PATCH] selftests/ftrace: Add test case for a symbol in a module without module name
From: Steven Rostedt @ 2026-07-14 14:42 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Shuah Khan, Andrii Nakryiko, linux-trace-kernel, bpf,
Francis Laniel, linux-kselftest
In-Reply-To: <169846405196.88147.17766692778800222203.stgit@devnote2>
Is this still a think, or can we remove it from patchwork?
https://patchwork.kernel.org/project/linux-trace-kernel/patch/169846405196.88147.17766692778800222203.stgit@devnote2/
-- Steve
On Sat, 28 Oct 2023 12:34:12 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Add a test case for probing on a symbol in a module without module name.
> When probing on a symbol in a module, ftrace accepts both the syntax that
> <MODNAME>:<SYMBOL> and <SYMBOL>. Current test case only checks the former
> syntax. This adds a test for the latter one.
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> .../ftrace/test.d/kprobe/kprobe_module.tc | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> index 7e74ee11edf9..4b32e1b9a8d3 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> @@ -13,6 +13,12 @@ fi
> MOD=trace_printk
> FUNC=trace_printk_irq_work
>
> +:;: "Add an event on a module function without module name" ;:
> +
> +echo "p:event0 $FUNC" > kprobe_events
> +test -d events/kprobes/event0 || exit_failure
> +echo "-:kprobes/event0" >> kprobe_events
> +
> :;: "Add an event on a module function without specifying event name" ;:
>
> echo "p $MOD:$FUNC" > kprobe_events
^ permalink raw reply
* Re: [PATCH v6 2/9] mm/page_owner: add MR_NEVER to enum migrate_reason and use it for last_migrate_reason
From: David Hildenbrand (Arm) @ 2026-07-14 14:44 UTC (permalink / raw)
To: Vlastimil Babka (SUSE), Ye Liu, Andrew Morton, Steven Rostedt,
Masami Hiramatsu, Jan Kiszka, Kieran Bingham
Cc: 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: <2911979b-193c-4cbc-85ed-f1d04a9977f0@kernel.org>
On 7/14/26 15:49, Vlastimil Babka (SUSE) wrote:
> On 7/14/26 15:22, David Hildenbrand (Arm) wrote:
>> On 7/14/26 15:09, Vlastimil Babka (SUSE) wrote:
>>>
>>> It's not used ever as a reason that would be actually passed to migration.
>>
>> Right, it's a placeholder for "there is no migrate reason because it is unset"
>>
>>> So I think the name is more descriptive this way.
>>
>> Not sure I agree. The usual translation of -1 -> unset is NONE or UNSET.
>
> (note it's no longer -1 after the patch.)
>
>> At least I was confused by "NEVER".
>
> I won't bikeshed this, so whatever.
Just to be clear: if everybody here agrees that MR_NEVER is the right thing to
use, fine with me.
I just stumbled over it and it caught my attention.
>
> Just to avoid another extra respin, please also say how the following line
> should change, as if we only rename MR_NEVER to MR_NONE, it will be:
>
> + EMe(MR_NONE, "never_migrated")
I would probably just have use "not set".
And if "last_migrate_reason == MR_NONE" that would imply "never".
Ye Liu, feel free to keep it as is if you agree that using NEVER is better here.
--
Cheers,
David
^ 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 15:34 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: <cc4797fa-6911-45a3-8775-69c2ef32a338@sirena.org.uk>
Hi Mark,
On 11-07-2026 02:56, Mark Brown wrote:
> On Sat, Jul 11, 2026 at 12:18:42AM +0530, Praveen Talari wrote:
>
>> The GENI SPI driver reports various transfer failures such as command
>> timeouts, DMA reset timeouts, DMA transaction errors, and unexpected
>> interrupt conditions. However, diagnosing the root cause of these
>> failures is difficult as the hardware state is not captured when the
>> error occurs.
>> +#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.
>
>> @@ -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?
I got you now.
Yes you are correct, its better trace before clearing it.
Will update in next patch.
Thanks,
Praveen Talari
^ permalink raw reply
* Re: [PATCH v3 4/5] sched: Add task enqueue/dequeue trace points
From: Steven Rostedt @ 2026-07-14 16:11 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Ingo Molnar, Peter Zijlstra, Nam Cao, linux-trace-kernel,
linux-kernel, Juri Lelli, Vincent Guittot, Masami Hiramatsu,
Mathieu Desnoyers, Dietmar Eggemann, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak
In-Reply-To: <61d904e5dc673d398c20bc4359e0f9b16ef4fde4.camel@redhat.com>
On Wed, 13 Aug 2025 08:50:39 +0200
Gabriele Monaco <gmonaco@redhat.com> wrote:
> On Mon, 2025-08-11 at 10:40 +0200, Nam Cao wrote:
> > Add trace points into enqueue_task() and dequeue_task(). They are
> > useful to implement RV monitor which validates RT scheduling.
> >
> > Signed-off-by: Nam Cao <namcao@linutronix.de>
> > ---
>
> Peter, Ingo, this patch adds new tracepoints in the scheduler do agree
> with the change, can we get an Ack?
Are we still waiting on an Ack for this?
I see it in the archive of Patchwork:
https://patchwork.kernel.org/project/linux-trace-kernel/list/?series=989907&state=%2A&archive=both
-- Steve
^ permalink raw reply
* Re: [PATCH 1/3] rv/reactors: fix lockdep "Invalid wait context" in rv_react()
From: Wen Yang @ 2026-07-14 17:39 UTC (permalink / raw)
To: Nam Cao, Thomas Weißschuh, Gabriele Monaco
Cc: linux-trace-kernel, linux-kernel
In-Reply-To: <87y0fkdojp.fsf@yellow.woof>
On 7/10/26 02:07, Nam Cao wrote:
> Wen Yang <wen.yang@linux.dev> writes:
>> How about a context-sensitive approach, eg:
>>
>> NMI/hardirq (interrupts masked, scheduler cannot run):
>> Keep LD_WAIT_FREE. The false positive cannot arise in this path,
>> and the original constraint against raw spinlocks is preserved where
>> it is meaningful.
>>
>> task/softirq/PREEMPT_RT irq thread (preemptible):
>> Raise to LD_WAIT_SPIN. Raw spinlocks become permitted — as Gabriele
>> note, this is a necessary consequence of any fix in this path.
>
> I don't get the point. Unless the lock's type is also context-sensitive,
> the reactors still cannot use raw spin locks.
>
Correct, and that is intentional. The patch is not about enabling raw
spinlocks in callbacks; it is about fixing a false positive in
preemptible context while preserving LD_WAIT_FREE where it is still
meaningful.
The lockdep check (kernel/locking/lockdep.c:4901) is:
if (next_outer > curr_inner) /* BUG: Invalid wait context */
The false positive in preemptible context:
rv_react_map (inner = LD_WAIT_FREE = 1) --> held by override map
timer interrupt
__schedule
rq->__lock (outer = LD_WAIT_SPIN = 2)
2 > 1 ---> BUG (while callback itself took no spinlock)
In NMI/hardirq context interrupts are masked; __schedule cannot run, so
rq->__lock is never acquired in this path. The false positive cannot
arise there, and LD_WAIT_FREE keeps raw spinlocks forbidden:
raw_spinlock_t (outer = LD_WAIT_SPIN = 2)
rv_react_map_atomic (inner = LD_WAIT_FREE = 1)
2 > 1 → BUG (intended: hardirq callbacks must be lock-free)
The commit message's "raw spinlocks in callbacks are permitted as a
necessary consequence" is imprecise -- that applies to the preemptible
path only. I will correct it in v2.
--
Best wishes,
Wen
^ permalink raw reply
* Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test
From: Wen Yang @ 2026-07-14 17:42 UTC (permalink / raw)
To: Gabriele Monaco, linux-trace-kernel, linux-kernel
Cc: Nam Cao, Thomas Weissschuh, Tomas Glozar, John Kacur,
Steven Rostedt, Shuah Khan, linux-kselftest
In-Reply-To: <b971748dea21a7a6eec3be8b95f4c18aec2d0552.camel@redhat.com>
On 7/10/26 16:31, Gabriele Monaco wrote:
> On Fri, 2026-07-10 at 01:08 +0800, Wen Yang wrote:
>>> load() { # returns true if there was a reaction
>>> - local lines_before num
>>> + local lines_before num load_pid ret
>>> num=$((($(nproc) + 1) / 2))
>>> lines_before=$(dmesg | wc -l)
>>> - stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q
>>> - dmesg | tail -n $((lines_before + 1)) | grep -q "rv: monitor wwnr
>>> does not allow event"
>>> + stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q &
>>> + load_pid=$!
>>> + timeout 5 dmesg -w | tail -n +$((lines_before + 1)) | grep -m 1 -q
>>> "rv: monitor wwnr does not allow event"
>>
>>
>> Minor nit: could we add a small delay (e.g., sleep 0.1) before dmesg?
>
> I don't get what the benefit would be.
>
> The intent of that line is to run a continuous dmesg (-w) for up to 5s (timeout)
> but stopping if an occurrence is found (grep -m 1 would close the pipe).
>
> Do you see a case in which this wouldn't happen?
>
You're right, the sleep is not needed. My concern was that a reaction
message generated in the brief window between starting stress-ng and the
dmesg pipeline being fully set up might be missed. But that can't
happen: dmesg -w always starts by flushing all currently buffered
messages before entering watch mode, so any reaction already in the ring
buffer will be included in its initial output. The tail -n +N offset
then correctly filters to only those messages beyond our pre-test
baseline.
Withdraw the nit. The Reviewed-by stands.
--
Best wishes,
Wen
>>
>> Reviewed-by: Wen Yang <wen.yang@linux.dev>
>>
^ permalink raw reply
* Re: [PATCH v3 14/17] verification/rvgen: Add selftests for rvgen kunit
From: Wen Yang @ 2026-07-14 17:49 UTC (permalink / raw)
To: Gabriele Monaco, linux-trace-kernel, linux-kernel
Cc: Nam Cao, Steven Rostedt, Thomas Weissschuh, Tomas Glozar,
John Kacur
In-Reply-To: <ed538f5f3a0976ea8a0be66b20a446442f7260c1.camel@redhat.com>
On 6/29/26 15:04, Gabriele Monaco wrote:
> On Mon, 2026-06-29 at 01:06 +0800, Wen Yang wrote:
>> On 6/25/26 20:14, Gabriele Monaco wrote:
>>> +static void handle_example_event(void *data, /* XXX: fill header */)
>>> +{
>>> + ltl_atom_update(task, LTL_EVENT_A, true/false);
>>> +}
>>> +
>>> +static int enable_test_ltl_kunit(void)
>>> +{
>>> + int retval;
>>> +
>>> + retval = ltl_monitor_init();
>>> + if (retval)
>>> + return retval;
>>> +
>>> + rv_attach_trace_probe("test_ltl_kunit", /* XXX: tracepoint */,
>>> handle_example_event);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void disable_test_ltl_kunit(void)
>>> +{
>>> + rv_detach_trace_probe("test_ltl_kunit", /* XXX: tracepoint */,
>>> handle_sample_event);
>>> +
>>
>> one typo:
>> handle_sample_event should be handle_example_event.
>
> Keep in mind that those files are not ready to build, users need to
> touch them anyway after generation from rvgen. Nevertheless, this has
> been inconsistent for a while and I should fix it.
>
Note that the same typo (handle_sample_event) appears in the other two
LTL golden files added in patch 05/17 as well: ltl_pertask.c and
test_ltl.c. The root is likely in the rvgen LTL template itself, so
the fix there would regenerate all three consistently.
>>> + ltl_monitor_destroy();
>>> +}
> ...
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_AUTHOR(/* TODO */);
>>
>> Please use a valid string here.
>
> Likewise, this is not supposed to build, we are just validating what
> rvgen produces and that's the expected output, the user will need to
> fill it with an appropriate string.
>
> LTL uses a different approach compared to DA/HA in this template, I'm not
> sure it's worth aligning the two..
>
Understood on the intent. One small concern: MODULE_AUTHOR(/* TODO */)
is subtly different from an XXX comment — a C preprocessor strips the
comment before macro expansion, leaving MODULE_AUTHOR() with an empty
argument. When the user does go to compile, they get a cryptic error
rather than a clear "fill this in" hint.
--
Best wishes,
Wen
^ permalink raw reply
* Re: [PATCH v3 04/17] tools/rv: Add selftests
From: Wen Yang @ 2026-07-14 18:01 UTC (permalink / raw)
To: Gabriele Monaco, linux-trace-kernel, linux-kernel
Cc: Nam Cao, Steven Rostedt, Thomas Weissschuh, Tomas Glozar,
John Kacur
In-Reply-To: <01b025c43dda6efe97646162e61002fdf5fff0e3.camel@redhat.com>
On 6/29/26 14:51, Gabriele Monaco wrote:
> Please cut down the context a bit more next time, it makes it much
> easier to find your review.
>
> On Mon, 2026-06-29 at 01:10 +0800, Wen Yang wrote:
>> On 6/25/26 20:14, Gabriele Monaco wrote:
>>> + eval "$TIMEOUT" "$command" &> check_output.$$ &
>>> + bgpid=$!
>>> + pid=$(pgrep -f "${command%%[|;&>]*}" | tail -n1)
>>
>> The pgrep runs may immediately after the background fork, before the
>> child process has had time to exec.
>
> Yeah I'm aware of this but kind of ignored it for now and never seen it
> making troubles in practice..
>
> I could add some delay waiting for the task like:
>
> while [ -z "$pid" ]; do
> sleep .5
> pid=$(pgrep -f "${command%%[|;&>]*}" | tail -n1)
> done
>
> With probably a maximum of some N retrials in case the task never
> started or we messed up the pattern.
> That may still race in case the command exits before we pgrep it, but in
> practice that shouldn't be a problem in our tests.
>
> Any better idea? We cannot really rely on the shell's $! because command
> is using a combination of eval+timer and we'd get the wrong pid.
>
- Since $bgpid is the timeout process, its direct child is exactly the
command we want. Using pgrep -P $bgpid avoids the fragile pattern
matching of pgrep -f and won't accidentally match unrelated
processes with a similar command string, eg:
for i in $(seq 10); do
pid=$(pgrep -P "$bgpid" | head -1)
[ -n "$pid" ] && break
sleep 0.5
done
Note: a bounded retry loop may be necessary; without an upper limit
the loop hangs indefinitely if the command fails to exec.
- For the verbose test specifically ("my pid is $pid"), the pid already
appears in rv's own output. An alternative is to match it with a
numeric pattern instead:
"my pid is [0-9]\+"
This sidesteps the race entirely for that test case.
--
Best wishes,
Wen
^ permalink raw reply
* [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption
From: Jinchao Wang @ 2026-07-14 18:22 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
Motivation
==========
The hardest memory corruption bugs are the silent ones: a rogue writer
scribbles over a live object through a stale pointer or a race, and
the victim crashes in a code path far away from the culprit. Any
single developer hits such a bug rarely, but across the kernel's code
base and install base they keep arriving, and each one is
disproportionately expensive to localize. The existing tools report
the symptom, not the writer:
- KASAN/KFENCE catch invalid accesses, but a targeted use-after-free
or an in-bounds logical overwrite (a *valid* pointer written at the
wrong time) never violates memory safety, so they stay silent - and
KASAN's rebuild, overhead and redzones often perturb the bug away.
- Hardware watchpoints via kgdb or perf can watch one fixed address,
system-wide, for the whole run. But the interesting address is
usually per-object and per-invocation ("field X of the object this
function is currently operating on"), which cannot be expressed.
Design
======
KWatch implements two key mechanisms: a function-scoped watch window
that decides when the hardware breakpoint is armed and released, and
a flexible expression engine that resolves which address it guards.
A watch is configured through debugfs with a single line; hits are
reported through a tracepoint, carrying the writing instruction and
a stack trace.
The window: a kretprobe pair opens the watch at function entry and
closes it on return; inside, a per-CPU hardware breakpoint from a
preallocated pool is re-pointed at the target address. The pool is
managed locklessly, so a window can open in whatever context the
watched function runs in - real NMI excepted - and a hit can fire
and be handled in any context.
The window is also what makes the scarce hardware affordable: x86
has only four hardware breakpoint slots per CPU, and every corruption
happens within some execution context, so a breakpoint is armed only
while that context runs. The rest of the system runs at full speed and
only the watched function pays the kprobe cost, which keeps KWatch
usable on busy, highly concurrent systems. Global variables can also
be watched without a window, in a time-bounded anchor session.
The expression engine: at each entry it evaluates the configured
watch expression to resolve that invocation's target address. The
base can be a function argument, the stack pointer, a symbol or an
absolute address; offsets and pointer dereferences chain on top, so
heap fields reachable from an argument, globals and stack slots are
all expressible - no objdump session needed.
KWatch is also designed for painless deployment: it is fully
self-contained and can be built as a module, loaded only when a
corruption hunt needs it. It is just a debugfs entry until a watch
is configured, then just a kprobe on the watched function, with the
breakpoint armed only when needed.
A real case: dummy_hcd
======================
Gadget requests were completing through a clobbered req->complete.
Months of KASAN-enabled syzkaller runs produced only downstream
symptoms, with no lead on the root cause. Watching the victim field
with KWatch:
func_name=usb_gadget_giveback_request watch_expr=arg2+56 \
watch_len=8 access_type=0
caught the writer in the act:
kwatch_hit: KWatch HIT: time=370.399836 ip=memcpy+0xc/0x30
addr=0xffff888109cf5218
=> usb_ep_queue+0xf1/0x3c0
=> raw_process_ep_io+0x5e4/0xd80
=> raw_ioctl+0x251c/0x41c0
=> __se_sys_ioctl+0xfc/0x170
=> do_syscall_64+0x174/0x580
=> entry_SYSCALL_64_after_hwframe+0x77/0x7f
on the same request that crashed an instant later - the crash RIP was
the just-written garbage value. Root cause: dummy_queue()'s single
shared fifo_req is struct-copied over while dummy_timer() is
mid-giveback. Neither a use-after-free nor list corruption, so KASAN
and CONFIG_DEBUG_LIST are blind to it by design. A fix based on this
diagnosis is under review [1] - KWatch's part was pinpointing the
root cause: who clobbers the pointer, and from where.
Series layout
=============
Patches 1-4: a minimal "reinstall" operation for hw_breakpoint.
Re-pointing an already-installed breakpoint from a kprobe handler is
not possible with the current API (register/unregister may sleep and
rebalances constraints); reinstall lets the arch rewrite a slot it
already owns, and modify_wide_hw_breakpoint_local() exposes that for
the local CPU - cross-CPU propagation is the caller's job (KWatch
uses async IPIs). Patch 4 is Masami Hiramatsu's work, carried
unchanged.
Patches 5-11: KWatch itself, in mm/kwatch/ (patch 7 exports
stack_trace_save_regs() for the modular build).
Patches 12-13: KUnit tests and documentation.
Testing
=======
The dummy_hcd hunt above exercised the function-window path against
a live reproducer. Global watching, session auto-stop and the KUnit
parser suite were verified end to end under QEMU on x86_64. Both
KWATCH=y and KWATCH=m build.
arm64
=====
This RFC deliberately targets x86 only. On arm64 the watchpoint
exception fires before the access, so the arch must single-step over
hits, and today it only does that for the default overflow handler.
Rather than hardcoding a KWatch hook into arm64 core code, I plan a
follow-up that adds a generic way for in-kernel breakpoint consumers
to request stepping, and arm64 support on top of it (a prototype
exists).
Relationship to KStackWatch
===========================
KWatch grew out of KStackWatch [2], an earlier tool aimed at stack
corruption only, and has been substantially reworked since. The
hw_breakpoint prerequisites are carried over from that series.
Major changes since the KStackWatch v8 posting:
- The watch expression engine widens the watchable range from the
stack to any address expressible via function arguments, globals
or stack addresses plus pointer dereference chains.
- The task_struct and scheduler hooks are gone; KWatch is now fully
self-contained, as described above.
- A time-bounded anchor context was added for watching global
variables (duration=N, auto-stop on expiry).
- Hits are reported through a tracepoint carrying a stack trace
instead of printk: safe in NMI-like contexts, and recoverable
after a crash (ftrace_dump_on_oops, kdump, pstore).
- Invocations in real NMI(-like) context are detected and rejected,
with a visible nmi_rejected counter.
- arm64 support and the auto-canary, profiling and test-module
extras were dropped from this first posting to keep it reviewable.
Feedback on the design, the implementation or the usage is welcome;
if you are staring at a corruption that KASAN and friends cannot
attribute, give KWatch a try, or simply Cc me - I am glad to help.
[1] https://lore.kernel.org/all/20260714064829.172098-1-wangjinchao600@gmail.com/
[2] https://lore.kernel.org/all/20251110163634.3686676-1-wangjinchao600@gmail.com/
Jinchao Wang (12):
arch: add HAVE_REINSTALL_HW_BREAKPOINT
x86/hw_breakpoint: Unify breakpoint install/uninstall
x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint
mm/kwatch: add watch expression parser and dereference engine
mm/kwatch: add lockless per-task context pool
stacktrace: export stack_trace_save_regs()
mm/kwatch: add hardware breakpoint backend
mm/kwatch: add probe lifecycle runtime
mm/kwatch: add anchor thread for global watchpoints
mm/kwatch: add debugfs control plane
mm/kwatch: add KUnit tests for the watch expression parser
Documentation/dev-tools: document KWatch
Masami Hiramatsu (Google) (1):
HWBP: Add modify_wide_hw_breakpoint_local() API
Documentation/dev-tools/index.rst | 1 +
Documentation/dev-tools/kwatch.rst | 193 +++++++++++++++
MAINTAINERS | 8 +
arch/Kconfig | 10 +
arch/x86/Kconfig | 1 +
arch/x86/include/asm/hw_breakpoint.h | 8 +
arch/x86/kernel/hw_breakpoint.c | 151 ++++++-----
include/linux/hw_breakpoint.h | 6 +
include/trace/events/kwatch.h | 57 +++++
kernel/events/hw_breakpoint.c | 37 +++
kernel/stacktrace.c | 2 +
mm/Kconfig | 1 +
mm/Makefile | 1 +
mm/kwatch/.kunitconfig | 9 +
mm/kwatch/Kconfig | 27 ++
mm/kwatch/Makefile | 4 +
mm/kwatch/anchor.c | 82 ++++++
mm/kwatch/core.c | 325 ++++++++++++++++++++++++
mm/kwatch/deref.c | 174 +++++++++++++
mm/kwatch/deref_test.c | 137 ++++++++++
mm/kwatch/hwbp.c | 358 +++++++++++++++++++++++++++
mm/kwatch/kwatch.h | 107 ++++++++
mm/kwatch/probe.c | 263 ++++++++++++++++++++
mm/kwatch/task_ctx.c | 105 ++++++++
24 files changed, 2005 insertions(+), 62 deletions(-)
create mode 100644 Documentation/dev-tools/kwatch.rst
create mode 100644 include/trace/events/kwatch.h
create mode 100644 mm/kwatch/.kunitconfig
create mode 100644 mm/kwatch/Kconfig
create mode 100644 mm/kwatch/Makefile
create mode 100644 mm/kwatch/anchor.c
create mode 100644 mm/kwatch/core.c
create mode 100644 mm/kwatch/deref.c
create mode 100644 mm/kwatch/deref_test.c
create mode 100644 mm/kwatch/hwbp.c
create mode 100644 mm/kwatch/kwatch.h
create mode 100644 mm/kwatch/probe.c
create mode 100644 mm/kwatch/task_ctx.c
--
2.53.0
^ permalink raw reply
* [RFC PATCH 01/13] arch: add HAVE_REINSTALL_HW_BREAKPOINT
From: Jinchao Wang @ 2026-07-14 18:22 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
In-Reply-To: <20260714182243.10687-1-wangjinchao600@gmail.com>
Some architectures can update the address, length or type of an
installed hardware breakpoint in place, without releasing and
re-reserving its slot. Add an opt-in Kconfig symbol so generic code
can offer such an operation on architectures that implement
arch_reinstall_hw_breakpoint().
This is a prerequisite for KWatch, which re-points preallocated
per-CPU breakpoints from atomic context, where the register/release
path (which may sleep and rebalances slot constraints) cannot be
used.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
arch/Kconfig | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index fa7507ac8e13..41b3784e0ddd 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -457,6 +457,16 @@ config HAVE_MIXED_BREAKPOINTS_REGS
Select this option if your arch implements breakpoints under the
latter fashion.
+config HAVE_REINSTALL_HW_BREAKPOINT
+ bool
+ depends on HAVE_HW_BREAKPOINT
+ help
+ Depending on the arch implementation of hardware breakpoints,
+ some of them are able to update the breakpoint configuration
+ without release and reserve the hardware breakpoint register.
+ What configuration is able to update depends on hardware and
+ software implementation.
+
config HAVE_USER_RETURN_NOTIFIER
bool
--
2.53.0
^ permalink raw reply related
* [RFC PATCH 02/13] x86/hw_breakpoint: Unify breakpoint install/uninstall
From: Jinchao Wang @ 2026-07-14 18:22 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
In-Reply-To: <20260714182243.10687-1-wangjinchao600@gmail.com>
Consolidate breakpoint management to reduce code duplication.
The diffstat was misleading, so the stripped code size is compared instead.
After refactoring, it is reduced from 11976 bytes to 11448 bytes on my
x86_64 system built with clang.
This also makes it easier to introduce arch_reinstall_hw_breakpoint().
In addition, including linux/types.h to fix a missing build dependency.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
arch/x86/include/asm/hw_breakpoint.h | 6 ++
arch/x86/kernel/hw_breakpoint.c | 141 +++++++++++++++------------
2 files changed, 84 insertions(+), 63 deletions(-)
diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index 0bc931cd0698..aa6adac6c3a2 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -5,6 +5,7 @@
#include <uapi/asm/hw_breakpoint.h>
#define __ARCH_HW_BREAKPOINT_H
+#include <linux/types.h>
/*
* The name should probably be something dealt in
@@ -18,6 +19,11 @@ struct arch_hw_breakpoint {
u8 type;
};
+enum bp_slot_action {
+ BP_SLOT_ACTION_INSTALL,
+ BP_SLOT_ACTION_UNINSTALL,
+};
+
#include <linux/kdebug.h>
#include <linux/percpu.h>
#include <linux/list.h>
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index f846c15f21ca..877509539300 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -49,7 +49,6 @@ static DEFINE_PER_CPU(unsigned long, cpu_debugreg[HBP_NUM]);
*/
static DEFINE_PER_CPU(struct perf_event *, bp_per_reg[HBP_NUM]);
-
static inline unsigned long
__encode_dr7(int drnum, unsigned int len, unsigned int type)
{
@@ -86,96 +85,112 @@ int decode_dr7(unsigned long dr7, int bpnum, unsigned *len, unsigned *type)
}
/*
- * Install a perf counter breakpoint.
- *
- * We seek a free debug address register and use it for this
- * breakpoint. Eventually we enable it in the debug control register.
- *
- * Atomic: we hold the counter->ctx->lock and we only handle variables
- * and registers local to this cpu.
+ * We seek a slot and change it or keep it based on the action.
+ * Returns slot number on success, negative error on failure.
+ * Must be called with IRQs disabled.
*/
-int arch_install_hw_breakpoint(struct perf_event *bp)
+static int manage_bp_slot(struct perf_event *bp, enum bp_slot_action action)
{
- struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- unsigned long *dr7;
- int i;
-
- lockdep_assert_irqs_disabled();
+ struct perf_event *old_bp;
+ struct perf_event *new_bp;
+ int slot;
+
+ switch (action) {
+ case BP_SLOT_ACTION_INSTALL:
+ old_bp = NULL;
+ new_bp = bp;
+ break;
+ case BP_SLOT_ACTION_UNINSTALL:
+ old_bp = bp;
+ new_bp = NULL;
+ break;
+ default:
+ return -EINVAL;
+ }
- for (i = 0; i < HBP_NUM; i++) {
- struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
+ for (slot = 0; slot < HBP_NUM; slot++) {
+ struct perf_event **curr = this_cpu_ptr(&bp_per_reg[slot]);
- if (!*slot) {
- *slot = bp;
- break;
+ if (*curr == old_bp) {
+ *curr = new_bp;
+ return slot;
}
}
- if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
- return -EBUSY;
+ if (old_bp) {
+ WARN_ONCE(1, "Can't find matching breakpoint slot");
+ return -EINVAL;
+ }
+
+ WARN_ONCE(1, "No free breakpoint slots");
+ return -EBUSY;
+}
+
+static void setup_hwbp(struct arch_hw_breakpoint *info, int slot, bool enable)
+{
+ unsigned long dr7;
- set_debugreg(info->address, i);
- __this_cpu_write(cpu_debugreg[i], info->address);
+ set_debugreg(info->address, slot);
+ __this_cpu_write(cpu_debugreg[slot], info->address);
- dr7 = this_cpu_ptr(&cpu_dr7);
- *dr7 |= encode_dr7(i, info->len, info->type);
+ dr7 = this_cpu_read(cpu_dr7);
+ if (enable)
+ dr7 |= encode_dr7(slot, info->len, info->type);
+ else
+ dr7 &= ~__encode_dr7(slot, info->len, info->type);
/*
- * Ensure we first write cpu_dr7 before we set the DR7 register.
- * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+ * Enabling:
+ * Ensure we first write cpu_dr7 before we set the DR7 register.
+ * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
*/
+ if (enable)
+ this_cpu_write(cpu_dr7, dr7);
+
barrier();
- set_debugreg(*dr7, 7);
+ set_debugreg(dr7, 7);
+
if (info->mask)
- amd_set_dr_addr_mask(info->mask, i);
+ amd_set_dr_addr_mask(enable ? info->mask : 0, slot);
- return 0;
+ /*
+ * Disabling:
+ * Ensure the write to cpu_dr7 is after we've set the DR7 register.
+ * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+ */
+ if (!enable)
+ this_cpu_write(cpu_dr7, dr7);
}
/*
- * Uninstall the breakpoint contained in the given counter.
- *
- * First we search the debug address register it uses and then we disable
- * it.
- *
- * Atomic: we hold the counter->ctx->lock and we only handle variables
- * and registers local to this cpu.
+ * find suitable breakpoint slot and set it up based on the action
*/
-void arch_uninstall_hw_breakpoint(struct perf_event *bp)
+static int arch_manage_bp(struct perf_event *bp, enum bp_slot_action action)
{
- struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- unsigned long dr7;
- int i;
+ struct arch_hw_breakpoint *info;
+ int slot;
lockdep_assert_irqs_disabled();
- for (i = 0; i < HBP_NUM; i++) {
- struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
-
- if (*slot == bp) {
- *slot = NULL;
- break;
- }
- }
-
- if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
- return;
+ slot = manage_bp_slot(bp, action);
+ if (slot < 0)
+ return slot;
- dr7 = this_cpu_read(cpu_dr7);
- dr7 &= ~__encode_dr7(i, info->len, info->type);
+ info = counter_arch_bp(bp);
+ setup_hwbp(info, slot, action != BP_SLOT_ACTION_UNINSTALL);
- set_debugreg(dr7, 7);
- if (info->mask)
- amd_set_dr_addr_mask(0, i);
+ return 0;
+}
- /*
- * Ensure the write to cpu_dr7 is after we've set the DR7 register.
- * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
- */
- barrier();
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+ return arch_manage_bp(bp, BP_SLOT_ACTION_INSTALL);
+}
- this_cpu_write(cpu_dr7, dr7);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp)
+{
+ arch_manage_bp(bp, BP_SLOT_ACTION_UNINSTALL);
}
static int arch_bp_generic_len(int x86_len)
--
2.53.0
^ permalink raw reply related
* [RFC PATCH 03/13] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint
From: Jinchao Wang @ 2026-07-14 18:22 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
In-Reply-To: <20260714182243.10687-1-wangjinchao600@gmail.com>
The new arch_reinstall_hw_breakpoint() function can be used in an
atomic context, unlike the more expensive free and re-allocation path.
This allows callers to efficiently re-establish an existing breakpoint,
and x86 advertises the capability via HAVE_REINSTALL_HW_BREAKPOINT.
Since a REINSTALL may change bp_len, setup_hwbp() must clear the
slot's stale len/type and enable bits in DR7 before re-encoding:
OR-merging the new encoding over the old one would keep the CPU
watching with the stale width (verified in QEMU by reading DR7 after
re-arming watch_len=1 over a len8 breakpoint: 0x999906aa merged
without the clearing, 0x199906aa with it).
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
arch/x86/Kconfig | 1 +
arch/x86/include/asm/hw_breakpoint.h | 2 ++
arch/x86/kernel/hw_breakpoint.c | 16 ++++++++++++++--
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index bdad90f210e4..5be698db0241 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -246,6 +246,7 @@ config X86
select HAVE_FUNCTION_TRACER
select HAVE_GCC_PLUGINS
select HAVE_HW_BREAKPOINT
+ select HAVE_REINSTALL_HW_BREAKPOINT
select HAVE_IOREMAP_PROT
select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index aa6adac6c3a2..c22cc4e87fc5 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -21,6 +21,7 @@ struct arch_hw_breakpoint {
enum bp_slot_action {
BP_SLOT_ACTION_INSTALL,
+ BP_SLOT_ACTION_REINSTALL,
BP_SLOT_ACTION_UNINSTALL,
};
@@ -65,6 +66,7 @@ extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
int arch_install_hw_breakpoint(struct perf_event *bp);
+int arch_reinstall_hw_breakpoint(struct perf_event *bp);
void arch_uninstall_hw_breakpoint(struct perf_event *bp);
void hw_breakpoint_pmu_read(struct perf_event *bp);
void hw_breakpoint_pmu_unthrottle(struct perf_event *bp);
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 877509539300..4221dbb899f9 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -100,6 +100,10 @@ static int manage_bp_slot(struct perf_event *bp, enum bp_slot_action action)
old_bp = NULL;
new_bp = bp;
break;
+ case BP_SLOT_ACTION_REINSTALL:
+ old_bp = bp;
+ new_bp = bp;
+ break;
case BP_SLOT_ACTION_UNINSTALL:
old_bp = bp;
new_bp = NULL;
@@ -134,10 +138,13 @@ static void setup_hwbp(struct arch_hw_breakpoint *info, int slot, bool enable)
__this_cpu_write(cpu_debugreg[slot], info->address);
dr7 = this_cpu_read(cpu_dr7);
+ /*
+ * Clear the slot's stale len/type and enable bits first: a REINSTALL
+ * with a different bp_len would otherwise OR-merge both encodings.
+ */
+ dr7 &= ~__encode_dr7(slot, 0xf, 0);
if (enable)
dr7 |= encode_dr7(slot, info->len, info->type);
- else
- dr7 &= ~__encode_dr7(slot, info->len, info->type);
/*
* Enabling:
@@ -188,6 +195,11 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
return arch_manage_bp(bp, BP_SLOT_ACTION_INSTALL);
}
+int arch_reinstall_hw_breakpoint(struct perf_event *bp)
+{
+ return arch_manage_bp(bp, BP_SLOT_ACTION_REINSTALL);
+}
+
void arch_uninstall_hw_breakpoint(struct perf_event *bp)
{
arch_manage_bp(bp, BP_SLOT_ACTION_UNINSTALL);
--
2.53.0
^ permalink raw reply related
* [RFC PATCH 04/13] HWBP: Add modify_wide_hw_breakpoint_local() API
From: Jinchao Wang @ 2026-07-14 18:30 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
In-Reply-To: <20260714182243.10687-1-wangjinchao600@gmail.com>
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Add modify_wide_hw_breakpoint_local() arch-wide interface which allows
hwbp users to update watch address on-line. This is available if the
arch supports CONFIG_HAVE_REINSTALL_HW_BREAKPOINT.
Note that this allows to change the type only for compatible types,
because it does not release and reserve the hwbp slot based on type.
For instance, you can not change HW_BREAKPOINT_W to HW_BREAKPOINT_X.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
include/linux/hw_breakpoint.h | 6 ++++++
kernel/events/hw_breakpoint.c | 37 +++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index db199d653dd1..ea373f2587f8 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -81,6 +81,9 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
perf_overflow_handler_t triggered,
void *context);
+extern int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+ struct perf_event_attr *attr);
+
extern int register_perf_hw_breakpoint(struct perf_event *bp);
extern void unregister_hw_breakpoint(struct perf_event *bp);
extern void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events);
@@ -124,6 +127,9 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
perf_overflow_handler_t triggered,
void *context) { return NULL; }
static inline int
+modify_wide_hw_breakpoint_local(struct perf_event *bp,
+ struct perf_event_attr *attr) { return -ENOSYS; }
+static inline int
register_perf_hw_breakpoint(struct perf_event *bp) { return -ENOSYS; }
static inline void unregister_hw_breakpoint(struct perf_event *bp) { }
static inline void
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 789add0c185a..20ca64f30508 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -888,6 +888,43 @@ void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events)
}
EXPORT_SYMBOL_GPL(unregister_wide_hw_breakpoint);
+/**
+ * modify_wide_hw_breakpoint_local - update breakpoint config for local CPU
+ * @bp: the hwbp perf event for this CPU
+ * @attr: the new attribute for @bp
+ *
+ * This does not release and reserve the slot of a HWBP; it just reuses the
+ * current slot on local CPU. So the users must update the other CPUs by
+ * themselves.
+ * Also, since this does not release/reserve the slot, this can not change the
+ * type to incompatible type of the HWBP.
+ * Return err if attr is invalid or the CPU fails to update debug register
+ * for new @attr.
+ */
+#ifdef CONFIG_HAVE_REINSTALL_HW_BREAKPOINT
+int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+ struct perf_event_attr *attr)
+{
+ int ret;
+
+ if (find_slot_idx(bp->attr.bp_type) != find_slot_idx(attr->bp_type))
+ return -EINVAL;
+
+ ret = hw_breakpoint_arch_parse(bp, attr, counter_arch_bp(bp));
+ if (ret)
+ return ret;
+
+ return arch_reinstall_hw_breakpoint(bp);
+}
+#else
+int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+ struct perf_event_attr *attr)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+EXPORT_SYMBOL_GPL(modify_wide_hw_breakpoint_local);
+
/**
* hw_breakpoint_is_used - check if breakpoints are currently used
*
--
2.53.0
^ permalink raw reply related
* [RFC PATCH 05/13] mm/kwatch: add watch expression parser and dereference engine
From: Jinchao Wang @ 2026-07-14 18:31 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
In-Reply-To: <20260714182243.10687-1-wangjinchao600@gmail.com>
KWatch watches a memory address that is only known once the target
function runs, e.g. "argument 1, plus 8, dereferenced once". Add the
two halves of that mechanism:
- kwatch_deref_parse() turns a textual watch expression
{base}[+-off][->[+-]off]... into a kwatch_config: a base anchor
(arg1..arg6, stack, an absolute address or - for built-in KWatch -
a symbol name) plus a static offset chain.
- kwatch_deref_resolve() replays the chain at probe time against
pt_regs. Every pointer load goes through get_kernel_nofault() and
the final address must be a kernel address.
Also add the internal kwatch.h header shared by the rest of the
series. Nothing is built yet; the Kconfig entry comes with the
control plane.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
mm/kwatch/Makefile | 3 +
mm/kwatch/deref.c | 174 +++++++++++++++++++++++++++++++++++++++++++++
mm/kwatch/kwatch.h | 107 ++++++++++++++++++++++++++++
3 files changed, 284 insertions(+)
create mode 100644 mm/kwatch/Makefile
create mode 100644 mm/kwatch/deref.c
create mode 100644 mm/kwatch/kwatch.h
diff --git a/mm/kwatch/Makefile b/mm/kwatch/Makefile
new file mode 100644
index 000000000000..69c21ae62123
--- /dev/null
+++ b/mm/kwatch/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_KWATCH) += kwatch.o
+
+kwatch-y := deref.o
diff --git a/mm/kwatch/deref.c b/mm/kwatch/deref.c
new file mode 100644
index 000000000000..a93c76139e7c
--- /dev/null
+++ b/mm/kwatch/deref.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/ptrace.h>
+#include <linux/sched.h>
+#include <linux/uaccess.h>
+#include <linux/kallsyms.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+
+#include "kwatch.h"
+
+int kwatch_deref_resolve(const struct kwatch_config *cfg, struct pt_regs *regs,
+ unsigned long *out_addr, u16 *out_len)
+{
+ unsigned long addr = 0;
+ int i;
+
+ /* 1. Resolve the Base Anchor */
+ if (cfg->base == KWATCH_BASE_STACK) {
+ addr = kernel_stack_pointer(regs);
+ if (unlikely(!addr))
+ return -EINVAL;
+ } else if (cfg->base >= KWATCH_BASE_ARG1 &&
+ cfg->base <= KWATCH_BASE_ARG6) {
+ int arg_idx = cfg->base - KWATCH_BASE_ARG1;
+
+ addr = regs_get_kernel_argument(regs, arg_idx);
+ } else if (cfg->base == KWATCH_BASE_ABS_ADDR ||
+ cfg->base == KWATCH_BASE_GLOBAL_SYM) {
+ /* Zero-latency load of the static symbol location */
+ addr = cfg->sym_addr;
+ } else {
+ return -EINVAL;
+ }
+
+ /* 2. The Pointer-Chasing FSM */
+ for (i = 0; i < cfg->offset_count; i++) {
+ addr += cfg->offsets[i];
+
+ if (i < cfg->offset_count - 1) {
+ unsigned long next_addr;
+
+ /* Dynamically read the pointer contents at runtime */
+ if (get_kernel_nofault(next_addr, (unsigned long *)addr))
+ return -EFAULT;
+
+ addr = next_addr;
+ }
+ }
+
+ /* Enforce strict Kernel-Space boundary */
+ if (unlikely(addr < TASK_SIZE_MAX))
+ return -EINVAL;
+
+ *out_addr = addr;
+ *out_len = cfg->watch_len;
+ return 0;
+}
+
+int kwatch_deref_parse(struct kwatch_config *cfg, const char *watch_expr)
+{
+ char *p, *sep, *dup_expr;
+ char type = '\0';
+ bool is_deref = false;
+ int ret = 0;
+
+ dup_expr = kstrdup(watch_expr, GFP_KERNEL);
+ if (!dup_expr)
+ return -ENOMEM;
+
+ cfg->offset_count = 1;
+ cfg->offsets[0] = 0;
+
+ /* 1. Isolate and Resolve Base Anchor */
+ p = dup_expr;
+ sep = NULL;
+ while (*p) {
+ if (*p == '+') {
+ sep = p;
+ type = '+';
+ break;
+ }
+ if (*p == '-') {
+ sep = p;
+ type = '-';
+ if (p[1] == '>')
+ is_deref = true;
+ break;
+ }
+ p++;
+ }
+
+ if (type)
+ *sep = '\0';
+
+ if (!strcmp(dup_expr, "stack")) {
+ cfg->base = KWATCH_BASE_STACK;
+ } else if (!strncmp(dup_expr, "arg", 3) && strlen(dup_expr) == 4) {
+ int arg_num;
+
+ if (kstrtoint(dup_expr + 3, 10, &arg_num) || arg_num < 1 ||
+ arg_num > 6) {
+ ret = -EINVAL;
+ goto out;
+ }
+ cfg->base = KWATCH_BASE_ARG1 + (arg_num - 1);
+ } else if (kstrtoul(dup_expr, 0, &cfg->sym_addr) == 0) {
+ cfg->base = KWATCH_BASE_ABS_ADDR;
+ } else {
+#if IS_BUILTIN(CONFIG_KWATCH)
+ cfg->sym_addr = kallsyms_lookup_name(dup_expr);
+ if (!cfg->sym_addr) {
+ pr_err("Failed to resolve symbol name: %s\n", dup_expr);
+ ret = -EINVAL;
+ goto out;
+ }
+ cfg->base = KWATCH_BASE_GLOBAL_SYM;
+#else
+ pr_err("cannot resolve symbol %s when built as a module, use a hex address\n",
+ dup_expr);
+ ret = -EINVAL;
+ goto out;
+#endif
+ }
+
+ if (!type)
+ goto out;
+
+ /* 2. Resolve Base Offset (if + or - exists) */
+ if (!is_deref) {
+ char *next;
+
+ *sep = type; /* Restore the '+' or '-' for kstrtol */
+ next = strstr(sep, "->");
+ if (next)
+ *next = '\0';
+
+ if (kstrtol(sep, 0, &cfg->offsets[0])) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ p = next ? next + 2 : NULL;
+ } else {
+ /* Jump directly to the first dereference after '->' */
+ p = sep + 2;
+ }
+
+ /* 3. Resolve Dereference Chain */
+ while (p) {
+ char *next;
+
+ if (cfg->offset_count >= MAX_DEREF_CHAIN) {
+ ret = -E2BIG;
+ goto out;
+ }
+
+ next = strstr(p, "->");
+ if (next)
+ *next = '\0';
+
+ if (kstrtol(p, 0, &cfg->offsets[cfg->offset_count++])) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ p = next ? next + 2 : NULL;
+ }
+
+out:
+ kfree(dup_expr);
+ return ret;
+}
diff --git a/mm/kwatch/kwatch.h b/mm/kwatch/kwatch.h
new file mode 100644
index 000000000000..e1ac8ae312f6
--- /dev/null
+++ b/mm/kwatch/kwatch.h
@@ -0,0 +1,107 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _MM_KWATCH_H
+#define _MM_KWATCH_H
+
+#include <linux/fprobe.h>
+#include <linux/kprobes.h>
+#include <linux/perf_event.h>
+#include <linux/sched.h>
+#include <linux/types.h>
+#include <linux/compiler.h>
+#include <linux/atomic.h>
+
+#define MAX_CONFIG_STR_LEN 512
+#define MAX_DEREF_CHAIN 4
+
+struct kwatch_watchpoint;
+
+struct kwatch_tsk_ctx {
+ struct task_struct *task;
+ struct kwatch_watchpoint *wp;
+ u16 depth;
+ u32 epoch;
+};
+
+struct kwatch_watchpoint {
+ struct perf_event *__percpu *event;
+ call_single_data_t __percpu *csd_arm;
+ call_single_data_t __percpu *csd_disarm;
+ struct perf_event_attr attr;
+ atomic_t in_use; // multi-consumer safe get/put
+ struct list_head list; // for cpu online and offline
+
+ struct task_struct *arm_tsk;
+ atomic_t pending_ipis;
+ atomic_t refcount;
+ bool teardown;
+};
+
+enum kwatch_access_type {
+ KWATCH_ACCESS_W,
+ KWATCH_ACCESS_R,
+ KWATCH_ACCESS_RW,
+ KWATCH_ACCESS_X,
+};
+
+enum kwatch_base_type {
+ KWATCH_BASE_STACK,
+ KWATCH_BASE_ABS_ADDR,
+ KWATCH_BASE_GLOBAL_SYM,
+ KWATCH_BASE_ARG1,
+ KWATCH_BASE_ARG2,
+ KWATCH_BASE_ARG3,
+ KWATCH_BASE_ARG4,
+ KWATCH_BASE_ARG5,
+ KWATCH_BASE_ARG6,
+};
+
+struct kwatch_config {
+ u16 max_watch;
+ char func_name[KSYM_NAME_LEN];
+ u16 func_offset;
+ u16 depth;
+ u16 duration;
+ enum kwatch_access_type access_type;
+ u16 watch_len;
+
+ /* Unified Deref Engine State */
+ enum kwatch_base_type base;
+ char watch_expr[MAX_CONFIG_STR_LEN];
+ unsigned long sym_addr;
+ long offsets[MAX_DEREF_CHAIN];
+ u8 offset_count;
+ u16 max_concurrency;
+};
+
+int kwatch_hwbp_prealloc(u16 max_watch, enum kwatch_access_type access_type);
+void kwatch_hwbp_free(void);
+int kwatch_hwbp_get(struct kwatch_watchpoint **out_wp);
+void kwatch_hwbp_arm(struct kwatch_watchpoint *wp, unsigned long addr, u16 len);
+int kwatch_hwbp_put(struct kwatch_watchpoint *wp);
+
+int kwatch_probe_start(struct kwatch_config *cfg);
+void kwatch_probe_stop(void);
+void kwatch_probe_mute(bool mute);
+bool kwatch_probe_validate_hit(struct pt_regs *regs, struct task_struct *arm_tsk);
+unsigned long kwatch_probe_nmi_rejected(void);
+
+int kwatch_tsk_ctx_prealloc(u16 max_concurrency);
+struct kwatch_tsk_ctx *kwatch_tsk_ctx_get(bool can_alloc);
+void kwatch_tsk_ctx_put(void);
+void kwatch_tsk_ctx_reset(struct kwatch_tsk_ctx *ctx, u32 new_epoch);
+void kwatch_tsk_ctx_release_wps(void);
+void kwatch_tsk_ctx_free(void);
+
+void kwatch_global_anchor(unsigned long duration_sec);
+int kwatch_anchor_start(u16 duration);
+void kwatch_anchor_stop(void);
+void kwatch_anchor_cancel_work(void);
+bool kwatch_anchor_has_expired(void);
+void kwatch_anchor_clear_expired(void);
+void kwatch_auto_stop(void);
+
+int kwatch_deref_resolve(const struct kwatch_config *cfg, struct pt_regs *regs,
+ unsigned long *out_addr, u16 *out_len);
+int kwatch_deref_parse(struct kwatch_config *cfg, const char *watch_expr);
+
+#endif /* _MM_KWATCH_H */
--
2.53.0
^ permalink raw reply related
* [RFC PATCH 06/13] mm/kwatch: add lockless per-task context pool
From: Jinchao Wang @ 2026-07-14 18:31 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
In-Reply-To: <20260714182243.10687-1-wangjinchao600@gmail.com>
A task that enters the watched function needs somewhere to keep its
window state (nesting depth, owned watchpoint, config epoch). The
lookup runs in kprobe and NMI-like contexts, so it must not allocate
or take locks.
Use a preallocated open-addressing array hashed by task_struct
pointer. Slots are claimed with cmpxchg() and released with
smp_store_release(); lookup is a read-only probe sequence. The pool
size (max_concurrency) bounds how many tasks can be inside watch
windows concurrently; excess tasks are simply not tracked.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
mm/kwatch/Makefile | 2 +-
mm/kwatch/task_ctx.c | 105 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+), 1 deletion(-)
create mode 100644 mm/kwatch/task_ctx.c
diff --git a/mm/kwatch/Makefile b/mm/kwatch/Makefile
index 69c21ae62123..cc6574df0d68 100644
--- a/mm/kwatch/Makefile
+++ b/mm/kwatch/Makefile
@@ -1,3 +1,3 @@
obj-$(CONFIG_KWATCH) += kwatch.o
-kwatch-y := deref.o
+kwatch-y := deref.o task_ctx.o
diff --git a/mm/kwatch/task_ctx.c b/mm/kwatch/task_ctx.c
new file mode 100644
index 000000000000..f8e582f0dcfe
--- /dev/null
+++ b/mm/kwatch/task_ctx.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/slab.h>
+#include <linux/hash.h>
+#include <linux/sched.h>
+#include <linux/log2.h>
+#include "kwatch.h"
+
+static u16 kwatch_ctx_pool_size;
+static u16 kwatch_ctx_pool_mask;
+
+static struct kwatch_tsk_ctx *kwatch_ctx_pool;
+
+int kwatch_tsk_ctx_prealloc(u16 max_concurrency)
+{
+ if (!max_concurrency)
+ max_concurrency = 256;
+
+ kwatch_ctx_pool_size = roundup_pow_of_two(max_concurrency);
+ kwatch_ctx_pool_mask = kwatch_ctx_pool_size - 1;
+
+ if (unlikely(!kwatch_ctx_pool)) {
+ kwatch_ctx_pool = kcalloc(kwatch_ctx_pool_size,
+ sizeof(struct kwatch_tsk_ctx),
+ GFP_KERNEL);
+ if (!kwatch_ctx_pool)
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+struct kwatch_tsk_ctx *kwatch_tsk_ctx_get(bool can_alloc)
+{
+ int start_idx, i, idx;
+ struct task_struct *t;
+
+ if (unlikely(!kwatch_ctx_pool))
+ return NULL;
+
+ start_idx = hash_ptr(current, ilog2(kwatch_ctx_pool_size));
+
+ for (i = 0; i < kwatch_ctx_pool_size; i++) {
+ idx = (start_idx + i) & kwatch_ctx_pool_mask;
+ t = READ_ONCE(kwatch_ctx_pool[idx].task);
+ if (t == current)
+ return &kwatch_ctx_pool[idx];
+ }
+
+ if (!can_alloc)
+ return NULL;
+
+ for (i = 0; i < kwatch_ctx_pool_size; i++) {
+ idx = (start_idx + i) & kwatch_ctx_pool_mask;
+ t = READ_ONCE(kwatch_ctx_pool[idx].task);
+ if (!t) {
+ if (!cmpxchg(&kwatch_ctx_pool[idx].task, NULL, current))
+ return &kwatch_ctx_pool[idx];
+ }
+ }
+
+ return NULL;
+}
+
+void kwatch_tsk_ctx_reset(struct kwatch_tsk_ctx *ctx, u32 new_epoch)
+{
+ struct kwatch_watchpoint *wp = xchg(&ctx->wp, NULL);
+
+ if (wp)
+ kwatch_hwbp_put(wp);
+ ctx->depth = 0;
+ ctx->epoch = new_epoch;
+}
+
+void kwatch_tsk_ctx_put(void)
+{
+ struct kwatch_tsk_ctx *ctx = kwatch_tsk_ctx_get(false);
+
+ if (unlikely(!ctx))
+ return;
+
+ kwatch_tsk_ctx_reset(ctx, 0);
+
+ /* Pairs with READ_ONCE() in kwatch_tsk_ctx_get() */
+ smp_store_release(&ctx->task, NULL);
+}
+
+void kwatch_tsk_ctx_release_wps(void)
+{
+ int i;
+
+ if (!kwatch_ctx_pool)
+ return;
+
+ for (i = 0; i < kwatch_ctx_pool_size; i++) {
+ struct kwatch_watchpoint *wp = xchg(&kwatch_ctx_pool[i].wp,
+ NULL);
+ if (wp)
+ kwatch_hwbp_put(wp);
+ }
+}
+
+void kwatch_tsk_ctx_free(void)
+{
+ kfree(kwatch_ctx_pool);
+ kwatch_ctx_pool = NULL;
+}
--
2.53.0
^ permalink raw reply related
* [RFC PATCH 07/13] stacktrace: export stack_trace_save_regs()
From: Jinchao Wang @ 2026-07-14 18:31 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
In-Reply-To: <20260714182243.10687-1-wangjinchao600@gmail.com>
The other stack_trace_save_*() flavours are exported, but the regs
variant is not, so no module can capture a stack trace for a given
pt_regs. KWatch, which may be built as a module, uses it to record
who wrote to a watched address from the hardware breakpoint handler.
Export it like its siblings.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/stacktrace.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
index afb3c116da91..d853c40f916b 100644
--- a/kernel/stacktrace.c
+++ b/kernel/stacktrace.c
@@ -175,6 +175,7 @@ unsigned int stack_trace_save_regs(struct pt_regs *regs, unsigned long *store,
arch_stack_walk(consume_entry, &c, current, regs);
return c.len;
}
+EXPORT_SYMBOL_GPL(stack_trace_save_regs);
#ifdef CONFIG_HAVE_RELIABLE_STACKTRACE
/**
@@ -325,6 +326,7 @@ unsigned int stack_trace_save_regs(struct pt_regs *regs, unsigned long *store,
save_stack_trace_regs(regs, &trace);
return trace.nr_entries;
}
+EXPORT_SYMBOL_GPL(stack_trace_save_regs);
#ifdef CONFIG_HAVE_RELIABLE_STACKTRACE
/**
--
2.53.0
^ permalink raw reply related
* [RFC PATCH 08/13] mm/kwatch: add hardware breakpoint backend
From: Jinchao Wang @ 2026-07-14 18:32 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
In-Reply-To: <20260714182243.10687-1-wangjinchao600@gmail.com>
Manage a preallocated pool of wide (per-CPU) perf hardware
breakpoints. All breakpoints are registered up front against a dummy
address; arming a watchpoint only re-points an already-registered
event, so the arm path can run from a kprobe handler.
- kwatch_hwbp_get()/put() claim and release pool entries with
per-slot cmpxchg, safe for concurrent consumers on any CPU.
- kwatch_hwbp_arm() updates the local CPU synchronously via
modify_wide_hw_breakpoint_local() and broadcasts asynchronous IPIs
to the other CPUs. Arm-side IPIs are rate-limited per CPU; disarm
IPIs are refcounted so an entry is only recycled once every CPU
has dropped it.
- Hits are reported through the kwatch:kwatch_hit tracepoint with a
short stack trace: the ftrace ring buffer is usable from NMI-like
context and survives a subsequent crash, unlike printk.
- A CPU hotplug callback creates/destroys the per-CPU events as CPUs
come and go.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
include/trace/events/kwatch.h | 57 ++++++
mm/kwatch/Makefile | 2 +-
mm/kwatch/hwbp.c | 358 ++++++++++++++++++++++++++++++++++
3 files changed, 416 insertions(+), 1 deletion(-)
create mode 100644 include/trace/events/kwatch.h
create mode 100644 mm/kwatch/hwbp.c
diff --git a/include/trace/events/kwatch.h b/include/trace/events/kwatch.h
new file mode 100644
index 000000000000..edb95405c386
--- /dev/null
+++ b/include/trace/events/kwatch.h
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM kwatch
+
+#if !defined(_TRACE_KWATCH_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_KWATCH_H
+
+#include <linux/tracepoint.h>
+#include <linux/ptrace.h>
+
+#define KWATCH_STACK_DEPTH 8
+
+struct trace_seq;
+const char *kwatch_trace_print_stack(struct trace_seq *p,
+ const unsigned long *stack,
+ unsigned int nr);
+
+TRACE_EVENT(kwatch_hit,
+ TP_PROTO(unsigned long ip, unsigned long sp, unsigned long addr,
+ u64 time_ns,
+ unsigned long *stack_entries, unsigned int stack_nr),
+ TP_ARGS(ip, sp, addr, time_ns, stack_entries, stack_nr),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, ip)
+ __field(unsigned long, sp)
+ __field(unsigned long, addr)
+ __field(u64, time_ns)
+ __field(unsigned int, stack_nr)
+ __array(unsigned long, stack, KWATCH_STACK_DEPTH)
+ ),
+
+ TP_fast_assign(
+ unsigned int i;
+
+ __entry->ip = ip;
+ __entry->sp = sp;
+ __entry->addr = addr;
+ __entry->time_ns = time_ns;
+ __entry->stack_nr = min_t(unsigned int, stack_nr,
+ KWATCH_STACK_DEPTH);
+ for (i = 0; i < __entry->stack_nr; i++)
+ __entry->stack[i] = stack_entries[i];
+ ),
+
+ TP_printk("KWatch HIT: time=%llu.%06lu ip=%pS addr=0x%lx%s",
+ __entry->time_ns / 1000000000ULL,
+ (unsigned long)((__entry->time_ns / 1000ULL) % 1000000ULL),
+ (void *)__entry->ip, __entry->addr,
+ kwatch_trace_print_stack(p, __entry->stack,
+ __entry->stack_nr))
+);
+
+#endif /* _TRACE_KWATCH_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/mm/kwatch/Makefile b/mm/kwatch/Makefile
index cc6574df0d68..b2bc3003c89b 100644
--- a/mm/kwatch/Makefile
+++ b/mm/kwatch/Makefile
@@ -1,3 +1,3 @@
obj-$(CONFIG_KWATCH) += kwatch.o
-kwatch-y := deref.o task_ctx.o
+kwatch-y := deref.o task_ctx.o hwbp.o
diff --git a/mm/kwatch/hwbp.c b/mm/kwatch/hwbp.c
new file mode 100644
index 000000000000..19498ba03826
--- /dev/null
+++ b/mm/kwatch/hwbp.c
@@ -0,0 +1,358 @@
+// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/cpuhotplug.h>
+#include <linux/ftrace.h>
+#include <linux/hw_breakpoint.h>
+#include <linux/sched/clock.h>
+#include <linux/irqflags.h>
+#include <linux/kallsyms.h>
+#include <linux/mutex.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/stacktrace.h>
+#include <linux/trace_seq.h>
+#include <linux/workqueue.h>
+
+#include "kwatch.h"
+
+static LIST_HEAD(kwatch_all_wp_list);
+static struct kwatch_watchpoint **kwatch_wp_slots;
+static u16 kwatch_wp_nr;
+static DEFINE_MUTEX(kwatch_all_wp_mutex);
+static unsigned long kwatch_dummy_holder __aligned(8);
+static int kwatch_hwbp_cpuhp_state = CPUHP_INVALID;
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/kwatch.h>
+
+/*
+ * Render the saved stack like the ftrace built-in stacktrace / dump_stack()
+ * style. Symbol resolution runs at trace read time, not in the hit path.
+ */
+const char *kwatch_trace_print_stack(struct trace_seq *p,
+ const unsigned long *stack,
+ unsigned int nr)
+{
+ const char *ret = trace_seq_buffer_ptr(p);
+ unsigned int i;
+
+ for (i = 0; i < nr; i++)
+ trace_seq_printf(p, "\n => %pS", (void *)stack[i]);
+ trace_seq_putc(p, 0);
+ return ret;
+}
+
+static void kwatch_hwbp_handler(struct perf_event *bp,
+ struct perf_sample_data *data,
+ struct pt_regs *regs)
+{
+ struct kwatch_watchpoint *wp = bp->overflow_handler_context;
+ unsigned long stack_entries[KWATCH_STACK_DEPTH];
+ unsigned int stack_nr;
+
+ if (!kwatch_probe_validate_hit(regs, wp->arm_tsk))
+ return;
+
+ stack_nr = stack_trace_save_regs(regs, stack_entries, KWATCH_STACK_DEPTH, 2);
+ trace_kwatch_hit(instruction_pointer(regs), kernel_stack_pointer(regs),
+ wp->attr.bp_addr, local_clock(),
+ stack_entries, stack_nr);
+}
+
+static void kwatch_hwbp_arm_local(void *info)
+{
+ struct kwatch_watchpoint *wp = info;
+ struct perf_event *bp;
+ unsigned long flags;
+ int cpu, err;
+
+ local_irq_save(flags);
+
+ cpu = smp_processor_id();
+ bp = per_cpu(*wp->event, cpu);
+
+ if (unlikely(!bp))
+ goto out;
+
+ kwatch_probe_mute(true);
+ barrier();
+
+ err = modify_wide_hw_breakpoint_local(bp, &wp->attr);
+ if (unlikely(err)) {
+ WARN_ONCE(1,
+ "KWatch: HWBP reinstall failed on CPU%d (err=%d, addr=0x%llx, len=%llu)\n",
+ cpu, err, wp->attr.bp_addr, wp->attr.bp_len);
+ }
+
+ barrier();
+ kwatch_probe_mute(false);
+
+out:
+ local_irq_restore(flags);
+}
+
+static inline void kwatch_hwbp_try_recycle(struct kwatch_watchpoint *wp)
+{
+ if (atomic_dec_and_test(&wp->pending_ipis)) {
+ if (!READ_ONCE(wp->teardown))
+ atomic_set_release(&wp->in_use, 0);
+
+ atomic_dec(&wp->refcount);
+ }
+}
+
+static void kwatch_hwbp_disarm_local(void *info)
+{
+ struct kwatch_watchpoint *wp = info;
+
+ kwatch_hwbp_arm_local(info);
+ kwatch_hwbp_try_recycle(wp);
+}
+
+static int kwatch_hwbp_cpu_online(unsigned int cpu)
+{
+ struct perf_event_attr attr;
+ struct kwatch_watchpoint *wp;
+ struct perf_event *bp;
+
+ mutex_lock(&kwatch_all_wp_mutex);
+ list_for_each_entry(wp, &kwatch_all_wp_list, list) {
+ attr = wp->attr;
+ attr.bp_addr = (unsigned long)&kwatch_dummy_holder;
+ bp = perf_event_create_kernel_counter(&attr, cpu, NULL,
+ kwatch_hwbp_handler, wp);
+ if (IS_ERR(bp)) {
+ pr_warn("%s failed to create watch on CPU %d: %ld\n",
+ __func__, cpu, PTR_ERR(bp));
+ continue;
+ }
+ per_cpu(*wp->event, cpu) = bp;
+ }
+ mutex_unlock(&kwatch_all_wp_mutex);
+ return 0;
+}
+
+static int kwatch_hwbp_cpu_offline(unsigned int cpu)
+{
+ struct kwatch_watchpoint *wp;
+ struct perf_event *bp;
+
+ mutex_lock(&kwatch_all_wp_mutex);
+ list_for_each_entry(wp, &kwatch_all_wp_list, list) {
+ bp = per_cpu(*wp->event, cpu);
+ if (bp) {
+ unregister_hw_breakpoint(bp);
+ per_cpu(*wp->event, cpu) = NULL;
+ }
+ }
+ mutex_unlock(&kwatch_all_wp_mutex);
+ return 0;
+}
+
+int kwatch_hwbp_get(struct kwatch_watchpoint **out_wp)
+{
+ struct kwatch_watchpoint *wp;
+ int i;
+
+ /*
+ * Per-slot cmpxchg claim: safe for concurrent consumers on any CPU,
+ * unlike llist_del_first() which requires a single consumer.
+ */
+ for (i = 0; i < kwatch_wp_nr; i++) {
+ wp = kwatch_wp_slots[i];
+ if (atomic_read(&wp->in_use))
+ continue;
+ if (atomic_cmpxchg(&wp->in_use, 0, 1) == 0) {
+ atomic_inc(&wp->refcount);
+ *out_wp = wp;
+ return 0;
+ }
+ }
+ return -EBUSY;
+}
+
+void kwatch_hwbp_arm(struct kwatch_watchpoint *wp, unsigned long addr, u16 len)
+{
+ static DEFINE_PER_CPU(u64, last_ipi_time);
+ int cur_cpu;
+ call_single_data_t *csd;
+ int cpu;
+ bool is_disarm = (addr == (unsigned long)&kwatch_dummy_holder);
+
+ wp->attr.bp_addr = addr;
+ wp->attr.bp_len = len;
+
+ if (!is_disarm)
+ wp->arm_tsk = current;
+
+ /* ensure attr update visible to other cpu before sending IPI */
+ smp_wmb();
+
+ atomic_set(&wp->pending_ipis, 1);
+ cur_cpu = get_cpu();
+
+ if (!is_disarm) {
+ u64 now = local_clock();
+ u64 last = this_cpu_read(last_ipi_time);
+
+ if (now - last < 1000000ULL) {
+ put_cpu();
+ return;
+ }
+ this_cpu_write(last_ipi_time, now);
+ }
+ for_each_online_cpu(cpu) {
+ if (cpu == cur_cpu)
+ continue;
+
+ if (is_disarm)
+ atomic_inc(&wp->pending_ipis);
+
+ csd = per_cpu_ptr(is_disarm ? wp->csd_disarm : wp->csd_arm,
+ cpu);
+ if (smp_call_function_single_async(cpu, csd) && is_disarm)
+ kwatch_hwbp_try_recycle(wp);
+ }
+ put_cpu();
+
+ if (is_disarm)
+ kwatch_hwbp_disarm_local(wp);
+ else
+ kwatch_hwbp_arm_local(wp);
+}
+
+int kwatch_hwbp_put(struct kwatch_watchpoint *wp)
+{
+ kwatch_hwbp_arm(wp, (unsigned long)&kwatch_dummy_holder,
+ sizeof(unsigned long));
+
+ return 0;
+}
+
+void kwatch_hwbp_free(void)
+{
+ struct kwatch_watchpoint *wp, *tmp;
+
+ kwatch_wp_nr = 0;
+ kfree(kwatch_wp_slots);
+ kwatch_wp_slots = NULL;
+
+ if (kwatch_hwbp_cpuhp_state != CPUHP_INVALID) {
+ cpuhp_remove_state_nocalls(kwatch_hwbp_cpuhp_state);
+ kwatch_hwbp_cpuhp_state = CPUHP_INVALID;
+ }
+
+ mutex_lock(&kwatch_all_wp_mutex);
+ list_for_each_entry_safe(wp, tmp, &kwatch_all_wp_list, list) {
+ list_del(&wp->list);
+
+ WRITE_ONCE(wp->teardown, true);
+ atomic_dec(&wp->refcount);
+
+ /* Wait for all async IPIs to finish */
+ while (atomic_read(&wp->refcount) > 0)
+ cpu_relax();
+
+ unregister_wide_hw_breakpoint(wp->event);
+ free_percpu(wp->csd_arm);
+ free_percpu(wp->csd_disarm);
+ kfree(wp);
+ }
+ mutex_unlock(&kwatch_all_wp_mutex);
+}
+
+int kwatch_hwbp_prealloc(u16 max_watch, enum kwatch_access_type access_type)
+{
+ struct kwatch_watchpoint *wp;
+ int success = 0, cpu;
+ u32 bp_type;
+ int ret;
+
+ switch (access_type) {
+ case KWATCH_ACCESS_X:
+ bp_type = HW_BREAKPOINT_X;
+ break;
+ case KWATCH_ACCESS_R:
+ bp_type = HW_BREAKPOINT_R;
+ break;
+ case KWATCH_ACCESS_RW:
+ bp_type = HW_BREAKPOINT_RW;
+ break;
+ case KWATCH_ACCESS_W:
+ default:
+ bp_type = HW_BREAKPOINT_W;
+ break;
+ }
+
+ while (!max_watch || success < max_watch) {
+ wp = kzalloc_obj(*wp);
+ if (!wp)
+ break;
+
+ wp->csd_arm = alloc_percpu(call_single_data_t);
+ wp->csd_disarm = alloc_percpu(call_single_data_t);
+ if (!wp->csd_arm || !wp->csd_disarm) {
+ free_percpu(wp->csd_arm);
+ free_percpu(wp->csd_disarm);
+ kfree(wp);
+ break;
+ }
+
+ for_each_possible_cpu(cpu) {
+ INIT_CSD(per_cpu_ptr(wp->csd_arm, cpu),
+ kwatch_hwbp_arm_local, wp);
+ INIT_CSD(per_cpu_ptr(wp->csd_disarm, cpu),
+ kwatch_hwbp_disarm_local, wp);
+ }
+
+ wp->teardown = false;
+
+ hw_breakpoint_init(&wp->attr);
+ wp->attr.bp_addr = (unsigned long)&kwatch_dummy_holder;
+ wp->attr.bp_len = sizeof(unsigned long);
+ wp->attr.bp_type = bp_type;
+
+ wp->event = register_wide_hw_breakpoint(&wp->attr,
+ kwatch_hwbp_handler,
+ wp);
+ if (IS_ERR((void *)wp->event)) {
+ free_percpu(wp->csd_arm);
+ free_percpu(wp->csd_disarm);
+ kfree(wp);
+ break;
+ }
+
+ atomic_set(&wp->refcount, 1);
+
+ mutex_lock(&kwatch_all_wp_mutex);
+ list_add(&wp->list, &kwatch_all_wp_list);
+ mutex_unlock(&kwatch_all_wp_mutex);
+ success++;
+ }
+
+ if (!success)
+ return -EBUSY;
+
+ kwatch_wp_slots = kcalloc(success, sizeof(*kwatch_wp_slots),
+ GFP_KERNEL);
+ if (!kwatch_wp_slots) {
+ kwatch_hwbp_free();
+ return -ENOMEM;
+ }
+ mutex_lock(&kwatch_all_wp_mutex);
+ list_for_each_entry(wp, &kwatch_all_wp_list, list)
+ kwatch_wp_slots[kwatch_wp_nr++] = wp;
+ mutex_unlock(&kwatch_all_wp_mutex);
+
+ ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "kwatch:online",
+ kwatch_hwbp_cpu_online,
+ kwatch_hwbp_cpu_offline);
+ if (ret < 0) {
+ kwatch_hwbp_free();
+ return ret;
+ }
+
+ kwatch_hwbp_cpuhp_state = ret;
+ return 0;
+}
--
2.53.0
^ permalink raw reply related
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