[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230322202449.512091-1-kan.liang@linux.intel.com>
Date: Wed, 22 Mar 2023 13:24:49 -0700
From: kan.liang@...ux.intel.com
To: peterz@...radead.org, mingo@...hat.com,
linux-kernel@...r.kernel.org
Cc: alexander.shishkin@...ux.intel.com, ak@...ux.intel.com,
adrian.hunter@...el.com, nadav.amit@...il.com,
Kan Liang <kan.liang@...ux.intel.com>,
Zhengjun Xing <zhengjun.xing@...ux.intel.com>
Subject: [PATCH V2] perf/core: Fix the same task check in perf_event_set_output
From: Kan Liang <kan.liang@...ux.intel.com>
The same task check in perf_event_set_output has some potential issues
for some usages.
For the current perf code, there is a problem if using of
perf_event_open() to have multiple samples getting into the same mmap’d
memory when they are both attached to the same process.
https://lore.kernel.org/all/92645262-D319-4068-9C44-2409EF44888E@gmail.com/
Because the event->ctx is not ready when the perf_event_set_output() is
invoked in the perf_event_open().
Besides the above issue, before the commit bd2756811766 ("perf: Rewrite
core context handling"), perf record can errors out when sampling with
a hardware event and a software event as below.
$ perf record -e cycles,dummy --per-thread ls
failed to mmap with 22 (Invalid argument)
That's because that prior to the commit a hardware event and a software
event are from different task context.
The problem should be a long time issue since commit c3f00c70276d
("perk: Separate find_get_context() from event initialization").
The task struct is stored in the event->hw.target for each per-thread
event. It is a more reliable way to determine whether two events are
attached to the same task.
The event->hw.target was also introduced several years ago by the
commit 50f16a8bf9d7 ("perf: Remove type specific target pointers"). It
can not only be used to fix the issue with the current code, but also
back port to fix the issues with an older kernel.
Note: The event->hw.target was introduced later than commit
c3f00c70276d. The patch may cannot be applied between the commit
c3f00c70276d and commit 50f16a8bf9d7. Anybody that wants to back-port
this at that period may have to find other solutions.
Fixes: c3f00c70276d ("perf: Separate find_get_context() from event initialization")
Reviewed-by: Zhengjun Xing <zhengjun.xing@...ux.intel.com>
Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
---
Changes since V1:
- Update the description to clarify the history, especially the changes
because of the rewrite.
- Update the Fixes to reflect the correct commit which introduced the
issue.
kernel/events/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 0c49183656fd..07e46cb098d1 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -12178,7 +12178,7 @@ perf_event_set_output(struct perf_event *event, struct perf_event *output_event)
/*
* If its not a per-cpu rb, it must be the same task.
*/
- if (output_event->cpu == -1 && output_event->ctx != event->ctx)
+ if (output_event->cpu == -1 && output_event->hw.target != event->hw.target)
goto out;
/*
--
2.35.1
Powered by blists - more mailing lists