From: Peter Zijlstra <peterz@infradead.org>
To: Yang Jihong <yangjihong1@huawei.com>
Cc: mingo@redhat.com, acme@kernel.org, mark.rutland@arm.com,
alexander.shishkin@linux.intel.com, jolsa@kernel.org,
namhyung@kernel.org, irogers@google.com, adrian.hunter@intel.com,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf/core: Fix perf_sample_data not properly initialized for different swevents in perf_tp_event()
Date: Mon, 17 Apr 2023 13:45:12 +0200 [thread overview]
Message-ID: <20230417114512.GK83892@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20230412095240.181516-1-yangjihong1@huawei.com>
On Wed, Apr 12, 2023 at 09:52:40AM +0000, Yang Jihong wrote:
> data->sample_flags may be modified in perf_prepare_sample(),
> in perf_tp_event(), different swevents use the same on-stack
> perf_sample_data, the previous swevent may change sample_flags in
> perf_prepare_sample(), as a result, some members of perf_sample_data are
> not correctly initialized when next swevent_event preparing sample
> (for example data->id, the value varies according to swevent).
>
> A simple scenario triggers this problem is as follows:
>
> # perf record -e sched:sched_switch --switch-output-event sched:sched_switch -a sleep 1
> [ perf record: dump data: Woken up 0 times ]
> [ perf record: Dump perf.data.2023041209014396 ]
> [ perf record: dump data: Woken up 0 times ]
> [ perf record: Dump perf.data.2023041209014662 ]
> [ perf record: dump data: Woken up 0 times ]
> [ perf record: Dump perf.data.2023041209014910 ]
> [ perf record: Woken up 0 times to write data ]
> [ perf record: Dump perf.data.2023041209015164 ]
> [ perf record: Captured and wrote 0.069 MB perf.data.<timestamp> ]
> # ls -l
> total 860
> -rw------- 1 root root 95694 Apr 12 09:01 perf.data.2023041209014396
> -rw------- 1 root root 606430 Apr 12 09:01 perf.data.2023041209014662
> -rw------- 1 root root 82246 Apr 12 09:01 perf.data.2023041209014910
> -rw------- 1 root root 82342 Apr 12 09:01 perf.data.2023041209015164
> # perf script -i perf.data.2023041209014396
> 0x11d58 [0x80]: failed to process type: 9 [Bad address]
>
> Solution: Add perf_sample_data_flags_{save, restore} helpers to save and
> restore sample_flags when processing different swevents
>
> After fix:
>
> # perf record -e sched:sched_switch --switch-output-event sched:sched_switch -a sleep 1
> [ perf record: dump data: Woken up 0 times ]
> [ perf record: Dump perf.data.2023041209442259 ]
> [ perf record: dump data: Woken up 0 times ]
> [ perf record: Dump perf.data.2023041209442514 ]
> [ perf record: dump data: Woken up 0 times ]
> [ perf record: Dump perf.data.2023041209442760 ]
> [ perf record: Woken up 0 times to write data ]
> [ perf record: Dump perf.data.2023041209443003 ]
> [ perf record: Captured and wrote 0.069 MB perf.data.<timestamp> ]
> # ls -l
> total 864
> -rw------- 1 root root 100166 Apr 12 09:44 perf.data.2023041209442259
> -rw------- 1 root root 606438 Apr 12 09:44 perf.data.2023041209442514
> -rw------- 1 root root 82246 Apr 12 09:44 perf.data.2023041209442760
> -rw------- 1 root root 82342 Apr 12 09:44 perf.data.2023041209443003
> # perf script -i perf.data.2023041209442259 | head -n 5
> perf 232 [000] 66.846217: sched:sched_switch: prev_comm=perf prev_pid=232 prev_prio=120 prev_state=D ==> next_comm=perf next_pid=234 next_prio=120
> perf 234 [000] 66.846449: sched:sched_switch: prev_comm=perf prev_pid=234 prev_prio=120 prev_state=S ==> next_comm=perf next_pid=232 next_prio=120
> perf 232 [000] 66.846546: sched:sched_switch: prev_comm=perf prev_pid=232 prev_prio=120 prev_state=R ==> next_comm=perf next_pid=234 next_prio=120
> perf 234 [000] 66.846606: sched:sched_switch: prev_comm=perf prev_pid=234 prev_prio=120 prev_state=S ==> next_comm=perf next_pid=232 next_prio=120
> perf 232 [000] 66.846646: sched:sched_switch: prev_comm=perf prev_pid=232 prev_prio=120 prev_state=R ==> next_comm=perf next_pid=234 next_prio=120
This seems a little bit short on analysis; what actual flags are the
problem? Much of the data will in fact be identical between these
invocations and endlessly re-computing that is wasteful.
I'm thinking perhaps those flags that update ->dyn_size are the problem?
At the same time, Should you not also then clear dyn_size?
next prev parent reply other threads:[~2023-04-17 11:46 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-12 9:52 [PATCH] perf/core: Fix perf_sample_data not properly initialized for different swevents in perf_tp_event() Yang Jihong
2023-04-17 11:45 ` Peter Zijlstra [this message]
2023-04-18 1:35 ` Yang Jihong
2023-04-18 10:25 ` Peter Zijlstra
2023-04-19 1:47 ` Yang Jihong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230417114512.GK83892@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=yangjihong1@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox