From: Leo Yan <leo.yan@linaro.org> To: German Gomez <german.gomez@arm.com> Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>, Mike Leach <mike.leach@linaro.org>, linux-arm-kernel@lists.infradead.org, coresight@lists.linaro.org, James Clark <james.clark@arm.com> Subject: Re: [PATCH 4/5] perf arm-spe: Implement find_snapshot callback Date: Wed, 20 Oct 2021 21:25:20 +0800 [thread overview] Message-ID: <20211020132520.GH49614@leoy-ThinkPad-X240s> (raw) In-Reply-To: <0661828d-f7d9-fd8f-2a57-19364d2e5218@arm.com> On Tue, Oct 19, 2021 at 06:34:24PM +0100, German Gomez wrote: > Hi Leo, > > On 17/10/2021 13:05, Leo Yan wrote: > > On Thu, Sep 16, 2021 at 04:46:34PM +0100, German Gomez wrote: > > > > [...] > > > > If run a test case (the test is pasted at the end of the reply), I > > can get quite different AUX trace data with passing different wait > > period before sending the first USR2 signal. > > > > # sh test_arm_spe_snapshot.sh 2 > > Couldn't synthesize bpf events. > > stress: info: [5768] dispatching hogs: 1 cpu, 0 io, 0 vm, 0 hdd > > [ perf record: Woken up 3 times to write data ] > > [ perf record: Captured and wrote 2.833 MB perf.data ] > > > > # sh test_arm_spe_snapshot.sh 10 > > Couldn't synthesize bpf events. > > stress: info: [5776] dispatching hogs: 1 cpu, 0 io, 0 vm, 0 hdd > > [ perf record: Woken up 3 times to write data ] > > [ perf record: Captured and wrote 24.356 MB perf.data ] > > > > The first command passes argument '2' so the test will wait for 2 > > seconds before send USR2 signal for snapshot, and the perf data file is > > 2.833 MB (so this means the Arm SPE trace data is about 2MB) for three > > snapshots. In the second command, the argument '10' means it will wait > > for 10 seconds before sending the USR2 signals, and every time it records > > the trace data from the full AUX buffer (8MB), at the end it gets 24MB > > AUX trace data. > > > > The issue happens in the second command, waiting for 10 seconds leads > > to the *full* AUX ring buffer is filled by Arm SPE, so the function > > arm_spe_buffer_has_wrapped() always return back true for this case. > > Afterwards, arm_spe_find_snapshot() doesn't respect the passed old > > header (from '*old') and assumes the trace data size is 'mm->len'. > > Returning the entire contents of the buffer once the first wrap-around > was detected was the intention of the patch, so I don't currently see it > as wrong. What were the values you were expecting to see in the test? I expect the second command takes three snapshots: the first time it should record AUX trace data with full buffer size (8MB) after waiting for 10 seconds, and later two times will take small AUX trace data since the interval (0.03s) is short and Arm SPE has not filled the full AUX buffer. > If the handling of snapshot mode by the perf tool can be improved after > upstreaming the changes to the driver, we could submit a followup patch > after that has been fixed. Okay, I understand now the main concern is for kernel driver changes, this patch for perf tool is fine for me: Reviewed-by: Leo Yan <leo.yan@linaro.org> Tested-by: Leo Yan <leo.yan@linaro.org> [...] > I will try these and the other driver changes and discuss them with the > team internally, thanks! Thanks a lot!
WARNING: multiple messages have this Message-ID (diff)
From: Leo Yan <leo.yan@linaro.org> To: German Gomez <german.gomez@arm.com> Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>, Mike Leach <mike.leach@linaro.org>, linux-arm-kernel@lists.infradead.org, coresight@lists.linaro.org, James Clark <james.clark@arm.com> Subject: Re: [PATCH 4/5] perf arm-spe: Implement find_snapshot callback Date: Wed, 20 Oct 2021 21:25:20 +0800 [thread overview] Message-ID: <20211020132520.GH49614@leoy-ThinkPad-X240s> (raw) In-Reply-To: <0661828d-f7d9-fd8f-2a57-19364d2e5218@arm.com> On Tue, Oct 19, 2021 at 06:34:24PM +0100, German Gomez wrote: > Hi Leo, > > On 17/10/2021 13:05, Leo Yan wrote: > > On Thu, Sep 16, 2021 at 04:46:34PM +0100, German Gomez wrote: > > > > [...] > > > > If run a test case (the test is pasted at the end of the reply), I > > can get quite different AUX trace data with passing different wait > > period before sending the first USR2 signal. > > > > # sh test_arm_spe_snapshot.sh 2 > > Couldn't synthesize bpf events. > > stress: info: [5768] dispatching hogs: 1 cpu, 0 io, 0 vm, 0 hdd > > [ perf record: Woken up 3 times to write data ] > > [ perf record: Captured and wrote 2.833 MB perf.data ] > > > > # sh test_arm_spe_snapshot.sh 10 > > Couldn't synthesize bpf events. > > stress: info: [5776] dispatching hogs: 1 cpu, 0 io, 0 vm, 0 hdd > > [ perf record: Woken up 3 times to write data ] > > [ perf record: Captured and wrote 24.356 MB perf.data ] > > > > The first command passes argument '2' so the test will wait for 2 > > seconds before send USR2 signal for snapshot, and the perf data file is > > 2.833 MB (so this means the Arm SPE trace data is about 2MB) for three > > snapshots. In the second command, the argument '10' means it will wait > > for 10 seconds before sending the USR2 signals, and every time it records > > the trace data from the full AUX buffer (8MB), at the end it gets 24MB > > AUX trace data. > > > > The issue happens in the second command, waiting for 10 seconds leads > > to the *full* AUX ring buffer is filled by Arm SPE, so the function > > arm_spe_buffer_has_wrapped() always return back true for this case. > > Afterwards, arm_spe_find_snapshot() doesn't respect the passed old > > header (from '*old') and assumes the trace data size is 'mm->len'. > > Returning the entire contents of the buffer once the first wrap-around > was detected was the intention of the patch, so I don't currently see it > as wrong. What were the values you were expecting to see in the test? I expect the second command takes three snapshots: the first time it should record AUX trace data with full buffer size (8MB) after waiting for 10 seconds, and later two times will take small AUX trace data since the interval (0.03s) is short and Arm SPE has not filled the full AUX buffer. > If the handling of snapshot mode by the perf tool can be improved after > upstreaming the changes to the driver, we could submit a followup patch > after that has been fixed. Okay, I understand now the main concern is for kernel driver changes, this patch for perf tool is fine for me: Reviewed-by: Leo Yan <leo.yan@linaro.org> Tested-by: Leo Yan <leo.yan@linaro.org> [...] > I will try these and the other driver changes and discuss them with the > team internally, thanks! Thanks a lot! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-10-20 13:25 UTC|newest] Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-16 15:46 [PATCH 1/5] perf cs-etm: Print size using consistent format German Gomez 2021-09-16 15:46 ` German Gomez 2021-09-16 15:46 ` [PATCH 2/5] perf arm-spe: " German Gomez 2021-09-16 15:46 ` German Gomez 2021-09-23 13:35 ` Leo Yan 2021-09-23 13:35 ` Leo Yan 2021-09-16 15:46 ` [PATCH 3/5] perf arm-spe: Add snapshot mode support German Gomez 2021-09-16 15:46 ` German Gomez 2021-10-20 12:48 ` Leo Yan 2021-10-20 12:48 ` Leo Yan 2021-09-16 15:46 ` [PATCH 4/5] perf arm-spe: Implement find_snapshot callback German Gomez 2021-09-16 15:46 ` German Gomez 2021-09-23 13:50 ` Leo Yan 2021-09-23 13:50 ` Leo Yan 2021-09-23 14:40 ` Leo Yan 2021-09-23 14:40 ` Leo Yan 2021-09-30 12:26 ` German Gomez 2021-09-30 12:26 ` German Gomez 2021-10-04 12:27 ` Leo Yan 2021-10-04 12:27 ` Leo Yan 2021-10-06 9:35 ` German Gomez 2021-10-06 9:35 ` German Gomez 2021-10-06 9:51 ` Leo Yan 2021-10-06 9:51 ` Leo Yan 2021-10-11 15:55 ` German Gomez 2021-10-11 15:55 ` German Gomez 2021-10-12 8:19 ` Will Deacon 2021-10-12 8:19 ` Will Deacon 2021-10-12 8:47 ` James Clark 2021-10-12 8:47 ` James Clark 2021-10-13 0:39 ` Leo Yan 2021-10-13 0:39 ` Leo Yan 2021-10-13 7:51 ` Will Deacon 2021-10-13 7:51 ` Will Deacon 2021-10-15 12:33 ` German Gomez 2021-10-15 12:33 ` German Gomez 2021-10-15 14:16 ` Leo Yan 2021-10-15 14:16 ` Leo Yan 2021-10-15 14:41 ` German Gomez 2021-10-15 14:41 ` German Gomez 2021-10-17 6:13 ` Leo Yan 2021-10-17 6:13 ` Leo Yan 2021-10-19 9:23 ` German Gomez 2021-10-19 9:23 ` German Gomez 2021-10-19 13:12 ` Leo Yan 2021-10-19 13:12 ` Leo Yan 2021-11-02 11:02 ` German Gomez 2021-11-02 11:02 ` German Gomez 2021-10-17 12:05 ` Leo Yan 2021-10-17 12:05 ` Leo Yan 2021-10-17 12:36 ` Leo Yan 2021-10-17 12:36 ` Leo Yan 2021-10-19 17:34 ` German Gomez 2021-10-19 17:34 ` German Gomez 2021-10-20 13:25 ` Leo Yan [this message] 2021-10-20 13:25 ` Leo Yan 2021-09-16 15:46 ` [PATCH 5/5] perf arm-spe: Snapshot mode test German Gomez 2021-09-16 15:46 ` German Gomez 2021-10-20 13:13 ` Leo Yan 2021-10-20 13:13 ` Leo Yan 2021-10-20 15:06 ` German Gomez 2021-10-20 15:06 ` German Gomez 2021-11-02 14:07 ` James Clark 2021-11-02 14:07 ` James Clark 2021-11-02 15:37 ` James Clark 2021-11-02 15:37 ` James Clark 2021-11-09 13:26 ` German Gomez 2021-11-09 13:26 ` German Gomez 2021-09-23 13:35 ` [PATCH 1/5] perf cs-etm: Print size using consistent format Leo Yan 2021-09-23 13:35 ` Leo Yan 2021-09-23 16:24 ` Mathieu Poirier 2021-09-23 16:24 ` Mathieu Poirier 2021-09-30 12:09 ` German Gomez 2021-09-30 12:09 ` German Gomez 2021-09-30 16:30 ` Mathieu Poirier 2021-09-30 16:30 ` Mathieu Poirier
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=20211020132520.GH49614@leoy-ThinkPad-X240s \ --to=leo.yan@linaro.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=coresight@lists.linaro.org \ --cc=german.gomez@arm.com \ --cc=james.clark@arm.com \ --cc=john.garry@huawei.com \ --cc=jolsa@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-perf-users@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mathieu.poirier@linaro.org \ --cc=mike.leach@linaro.org \ --cc=namhyung@kernel.org \ --cc=will@kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.