Hi Leo,

Would you be ok with the current patch the way it is? In case it's of
any help, I'm sharing the testing steps that James and I went through
when testing this internally, if you want to add to it

- Test that only a portion of the buffer is saved until there is a wraparound

$ ./perf record -vvv -e arm_spe/period=148576/u -S -- taskset --cpu-list 0 stress --cpu 1 & while true; do sleep 0.2; killall -s USR2 perf; done

- Test snapshot mode in CPU mode

$ sudo ./perf record -vvv -C 0 -e arm_spe/period=148576/u -S -- taskset --cpu-list 0 stress --cpu 1 &

- Test that auxtrace buffers correspond to an aux record
- Test snapshot default sizes in sudo and user modes
- Test small snapshot size

$ ./perf record -vvv -e arm_spe/period=148576/u -S1000 -m16,16 -- taskset --cpu-list 0 stress --cpu 1 &

If there are any concerns with the patches, please let me know and I
will try to address them.

Thanks,
German

On 13/10/2021 08:51, Will Deacon wrote:

On Wed, Oct 13, 2021 at 08:39:16AM +0800, Leo Yan wrote:

quoted

On Mon, Oct 11, 2021 at 04:55:37PM +0100, German Gomez wrote:

quoted

On 06/10/2021 10:51, Leo Yan wrote:

quoted

On Wed, Oct 06, 2021 at 10:35:20AM +0100, German Gomez wrote:

[...]

quoted

quoted

So simply say, I think the head pointer monotonically increasing is
the right thing to do in Arm SPE driver.

I will talk to James about how we can proceed on this.

Thanks!

I took this offline with James and, though it looks possible to patch
the SPE driver to have a monotonically increasing head pointer in order
to simplify the handling in the perf tool, it could be a breaking change
for users of the perf_event_open syscall that currently rely on the way
it works now.

Here I cannot create the connection between AUX head pointer and the
breakage of calling perf_event_open().

Could you elaborate what's the reason the monotonical increasing head
pointer will lead to the breakage for perf_event_open()?

It's a user-visible change in behaviour, isn't it? Therefore we risk
breaking applications that rely on the current behaviour if we change it
unconditionally.

Given that the driver has always worked like this and it doesn't sound like
it's the end of the world to deal with it in userspace (after all, it's
aligned with intel-pt), then I don't think we should change it.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel