Messages in this thread | | | Subject | Re: [PATCH v2 07/10] coresight: trbe: Do not truncate buffer on IRQ | From | Suzuki K Poulose <> | Date | Wed, 28 Jul 2021 10:25:49 +0100 |
| |
On 07/27/2021 02:06 PM, Suzuki K Poulose wrote: > On 27/07/2021 11:46, Mike Leach wrote: >> HI Suzuki, >> >> On Mon, 26 Jul 2021 at 17:01, Suzuki K Poulose <suzuki.poulose@arm.com> wrote: >>> >>> Hi Mike, >>> >>> On 26/07/2021 13:34, Mike Leach wrote: >>>> Hi Suzuki, >>>> >>>> On Fri, 23 Jul 2021 at 13:46, Suzuki K Poulose <suzuki.poulose@arm.com> wrote: >>>>> >>>>> The TRBE driver marks the AUX buffer as TRUNCATED when we get an IRQ >>>>> on FILL event. This has rather unwanted side-effect of the event >>>>> being disabled when there may be more space in the ring buffer. >>>>> >>>>> So, instead of TRUNCATE we need a different flag to indicate >>>>> that the trace may have lost a few bytes (i.e from the point of >>>>> generating the FILL event until the IRQ is consumed). Anyways, the >>>>> userspace must use the size from RECORD_AUX headers to restrict >>>>> the "trace" decoding. >>>>> >>>>> Using PARTIAL flag causes the perf tool to generate the >>>>> following warning: >>>>> >>>>> Warning: >>>>> AUX data had gaps in it XX times out of YY! >>>>> >>>>> Are you running a KVM guest in the background? >>>>> >>>>> which is pointlessly scary for a user. The other remaining options >>>>> are : >>>>> - COLLISION - Use by SPE to indicate samples collided >>>>> - Add a new flag - Specifically for CoreSight, doesn't sound >>>>> so good, if we can re-use something. >>>>> >>>> >>>> What is the user visible behaviour when using COLLISION? >>> >>> If you meant a Warning from the perf tool (similar to TRUNCATE or >>> PARTIAL), the answer is none. We could add one in the perf tool >>> if you think this is necessary. >>> >> >> I do - the problem is that we have replaced a visible warning with a >> silent failure. >> >> While we agree that the side effects of TRUNCATE mean it unfeasible as >> a solution here - at least the PARTIAL message does give some >> indication. >> The average perf user is going to rely on the output from the tool - >> if there is no warning they will assume all is good, but they have >> possible non-contiguous trace and no indication of such. >> >> Since we are using a collision flag in a particular context - i.e. >> coresight trace - we have the chance to provide an appropriate message >> for this context. >> >>>> The TRUNCATE warning is at least accurate - even if the KVM thing is >>>> something of a red herring. >>> >> >> Sorry - I meant PARTIAL here - but the comment stands otherwise. >> >>> >>>> It is easier to explain a "scary" warning, than try to debug someones >>>> problems if perf is silent or misleading when using the COLLISION >>>> flag. >>> >>> The RECORD_AUX still has this flag. So, if someone really wanted to >>> know how many times the TRBE fired the IRQ and thus potentially lost a >>> few bytes of the trace, they could always look at this. >>> >> >> They could - but how would they know that they needed to - what >> indicators would they have that the trace was not continuous? >> The point of the perf tool is that it presents an accurate picture to >> the user, based on the data collected. Most users aren't going to >> start digging into the intricacies of the perf data file formats and >> nor should they have to. >> >>> Definitely this is not something similar to "TRUNCATED", which we >>> realized the hard way, nor the PARTIAL. But the perf tool could >>> report something similar. Please remember that the perf tool always >>> uses the "size" field from the RECORD_AUX to limit the trace decoding. >>> >>> So, I am not sure how this could create new problems. >>> >> >> There is no issue with decode - but if a user is investigating a >> problem using trace, they need to be aware that some trace might be >> dropped. >> That way they can take mitigating action. > > Agreed. This is something that can be done by the perf tool.
fyi, I have posted a patch to do this here :
[0] https://lkml.kernel.org/r/20210728091219.527886-1-suzuki.poulose@arm.com
Suzuki
> > Kind regards > Suzuki
| |