| Message ID | 20200819084751.17686-1-leo.yan@linaro.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | f5f8e7e55fbdb4fdddec73518e23c48083108fbb |
| Headers | show |
| Series | [1/2] perf cs-etm: Fix corrupt data after perf inject from | expand |
Hi Leo and Al, On Wed, Aug 19, 2020 at 04:47:50PM +0800, Leo Yan wrote: > From: Al Grant <al.grant@arm.com> > > Commit 42bbabed09ce ("perf tools: Add hw_idx in struct branch_stack") > changed the format of branch stacks in perf samples. When samples use > this new format, a flag must be set in the corresponding event. > Synthesized branch stacks generated from CoreSight ETM trace were using > the new format, but not setting the event attribute, leading to > consumers seeing corrupt data. This patch fixes the issue by setting the > event attribute to indicate use of the new format. > > Fixes: 42bbabed09ce ("perf tools: Add hw_idx in struct branch_stack") > Signed-off-by: Al Grant <al.grant@arm.com> > Reviewed-by: Andrea Brunato <andrea.brunato@arm.com> > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > tools/perf/util/cs-etm.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index c283223fb31f..a2a369e2fbb6 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -1344,8 +1344,15 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm, > attr.sample_type &= ~(u64)PERF_SAMPLE_ADDR; > } > > - if (etm->synth_opts.last_branch) > + if (etm->synth_opts.last_branch) { > attr.sample_type |= PERF_SAMPLE_BRANCH_STACK; > + /* > + * We don't use the hardware index, but the sample generation > + * code uses the new format branch_stack with this field, > + * so the event attributes must indicate that it's present. > + */ > + attr.branch_sample_type |= PERF_SAMPLE_BRANCH_HW_INDEX; > + } I've see this patch before... I thought it had been merged - what happened? Thanks, Mathieu > > if (etm->synth_opts.instructions) { > attr.config = PERF_COUNT_HW_INSTRUCTIONS; > -- > 2.17.1 >
Hi Mathieu, On Thu, Aug 27, 2020 at 02:53:54PM -0600, Mathieu Poirier wrote: > Hi Leo and Al, > > On Wed, Aug 19, 2020 at 04:47:50PM +0800, Leo Yan wrote: > > From: Al Grant <al.grant@arm.com> > > > > Commit 42bbabed09ce ("perf tools: Add hw_idx in struct branch_stack") > > changed the format of branch stacks in perf samples. When samples use > > this new format, a flag must be set in the corresponding event. > > Synthesized branch stacks generated from CoreSight ETM trace were using > > the new format, but not setting the event attribute, leading to > > consumers seeing corrupt data. This patch fixes the issue by setting the > > event attribute to indicate use of the new format. > > > > Fixes: 42bbabed09ce ("perf tools: Add hw_idx in struct branch_stack") > > Signed-off-by: Al Grant <al.grant@arm.com> > > Reviewed-by: Andrea Brunato <andrea.brunato@arm.com> > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > --- > > tools/perf/util/cs-etm.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > > index c283223fb31f..a2a369e2fbb6 100644 > > --- a/tools/perf/util/cs-etm.c > > +++ b/tools/perf/util/cs-etm.c > > @@ -1344,8 +1344,15 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm, > > attr.sample_type &= ~(u64)PERF_SAMPLE_ADDR; > > } > > > > - if (etm->synth_opts.last_branch) > > + if (etm->synth_opts.last_branch) { > > attr.sample_type |= PERF_SAMPLE_BRANCH_STACK; > > + /* > > + * We don't use the hardware index, but the sample generation > > + * code uses the new format branch_stack with this field, > > + * so the event attributes must indicate that it's present. > > + */ > > + attr.branch_sample_type |= PERF_SAMPLE_BRANCH_HW_INDEX; > > + } > > I've see this patch before... I thought it had been merged - what happened? This patch before has been sent by Al to CoreSight mailing list but has not sent to LKML, this is why I resent it to LKML in case it's missed. Thanks, Leo
Em Mon, Aug 31, 2020 at 08:04:32AM +0800, Leo Yan escreveu: > Hi Mathieu, > > On Thu, Aug 27, 2020 at 02:53:54PM -0600, Mathieu Poirier wrote: > > Hi Leo and Al, > > > > On Wed, Aug 19, 2020 at 04:47:50PM +0800, Leo Yan wrote: > > > From: Al Grant <al.grant@arm.com> > > > > > > Commit 42bbabed09ce ("perf tools: Add hw_idx in struct branch_stack") > > > changed the format of branch stacks in perf samples. When samples use > > > this new format, a flag must be set in the corresponding event. > > > Synthesized branch stacks generated from CoreSight ETM trace were using > > > the new format, but not setting the event attribute, leading to > > > consumers seeing corrupt data. This patch fixes the issue by setting the > > > event attribute to indicate use of the new format. > > > > > > Fixes: 42bbabed09ce ("perf tools: Add hw_idx in struct branch_stack") > > > Signed-off-by: Al Grant <al.grant@arm.com> > > > Reviewed-by: Andrea Brunato <andrea.brunato@arm.com> > > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > > --- > > > tools/perf/util/cs-etm.c | 9 ++++++++- > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > > > index c283223fb31f..a2a369e2fbb6 100644 > > > --- a/tools/perf/util/cs-etm.c > > > +++ b/tools/perf/util/cs-etm.c > > > @@ -1344,8 +1344,15 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm, > > > attr.sample_type &= ~(u64)PERF_SAMPLE_ADDR; > > > } > > > > > > - if (etm->synth_opts.last_branch) > > > + if (etm->synth_opts.last_branch) { > > > attr.sample_type |= PERF_SAMPLE_BRANCH_STACK; > > > + /* > > > + * We don't use the hardware index, but the sample generation > > > + * code uses the new format branch_stack with this field, > > > + * so the event attributes must indicate that it's present. > > > + */ > > > + attr.branch_sample_type |= PERF_SAMPLE_BRANCH_HW_INDEX; > > > + } > > > > I've see this patch before... I thought it had been merged - what happened? > > This patch before has been sent by Al to CoreSight mailing list but has > not sent to LKML, this is why I resent it to LKML in case it's missed. So, was it Acked on the CoreSight mailing list? Are we missing any Acked-by or Reviewed-by for this 1/2 patch as we got for 2/2? - Arnaldo
Hi Arnaldo, On Tue, Sep 01, 2020 at 11:54:32AM -0300, Arnaldo Carvalho de Melo wrote: > Em Mon, Aug 31, 2020 at 08:04:32AM +0800, Leo Yan escreveu: > > Hi Mathieu, > > > > On Thu, Aug 27, 2020 at 02:53:54PM -0600, Mathieu Poirier wrote: > > > Hi Leo and Al, > > > > > > On Wed, Aug 19, 2020 at 04:47:50PM +0800, Leo Yan wrote: > > > > From: Al Grant <al.grant@arm.com> > > > > > > > > Commit 42bbabed09ce ("perf tools: Add hw_idx in struct branch_stack") > > > > changed the format of branch stacks in perf samples. When samples use > > > > this new format, a flag must be set in the corresponding event. > > > > Synthesized branch stacks generated from CoreSight ETM trace were using > > > > the new format, but not setting the event attribute, leading to > > > > consumers seeing corrupt data. This patch fixes the issue by setting the > > > > event attribute to indicate use of the new format. > > > > > > > > Fixes: 42bbabed09ce ("perf tools: Add hw_idx in struct branch_stack") > > > > Signed-off-by: Al Grant <al.grant@arm.com> > > > > Reviewed-by: Andrea Brunato <andrea.brunato@arm.com> > > > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > > > --- > > > > tools/perf/util/cs-etm.c | 9 ++++++++- > > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > > > > index c283223fb31f..a2a369e2fbb6 100644 > > > > --- a/tools/perf/util/cs-etm.c > > > > +++ b/tools/perf/util/cs-etm.c > > > > @@ -1344,8 +1344,15 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm, > > > > attr.sample_type &= ~(u64)PERF_SAMPLE_ADDR; > > > > } > > > > > > > > - if (etm->synth_opts.last_branch) > > > > + if (etm->synth_opts.last_branch) { > > > > attr.sample_type |= PERF_SAMPLE_BRANCH_STACK; > > > > + /* > > > > + * We don't use the hardware index, but the sample generation > > > > + * code uses the new format branch_stack with this field, > > > > + * so the event attributes must indicate that it's present. > > > > + */ > > > > + attr.branch_sample_type |= PERF_SAMPLE_BRANCH_HW_INDEX; > > > > + } > > > > > > I've see this patch before... I thought it had been merged - what happened? > > > > This patch before has been sent by Al to CoreSight mailing list but has > > not sent to LKML, this is why I resent it to LKML in case it's missed. > > So, was it Acked on the CoreSight mailing list? Are we missing any > Acked-by or Reviewed-by for this 1/2 patch as we got for 2/2? The CoreSight mailing list has some discussion for this patch set, when respin this patch set, I confirmed we don't miss any 'Acked' or 'Reviewed' tags. Thanks, Leo
Em Wed, Sep 02, 2020 at 08:39:32AM +0800, Leo Yan escreveu: > On Tue, Sep 01, 2020 at 11:54:32AM -0300, Arnaldo Carvalho de Melo wrote: > > Em Mon, Aug 31, 2020 at 08:04:32AM +0800, Leo Yan escreveu: > > > On Thu, Aug 27, 2020 at 02:53:54PM -0600, Mathieu Poirier wrote: > > > > On Wed, Aug 19, 2020 at 04:47:50PM +0800, Leo Yan wrote: > > > > I've see this patch before... I thought it had been merged - what happened? > > > This patch before has been sent by Al to CoreSight mailing list but has > > > not sent to LKML, this is why I resent it to LKML in case it's missed. > > So, was it Acked on the CoreSight mailing list? Are we missing any > > Acked-by or Reviewed-by for this 1/2 patch as we got for 2/2? > The CoreSight mailing list has some discussion for this patch set, > when respin this patch set, I confirmed we don't miss any 'Acked' or > 'Reviewed' tags. So, this patch was included in today's perf-tools-fixes-for-v5.9-2020-09-01 signed tag sent to Linus in a pull req, please update your perf/urgent branch, and go on from there, I'll merge that into my perf/core branch as soon as Linus merges it. - Arnaldo
On Tue, Sep 01, 2020 at 10:47:54PM -0300, Arnaldo Carvalho de Melo wrote: > Em Wed, Sep 02, 2020 at 08:39:32AM +0800, Leo Yan escreveu: > > On Tue, Sep 01, 2020 at 11:54:32AM -0300, Arnaldo Carvalho de Melo wrote: > > > Em Mon, Aug 31, 2020 at 08:04:32AM +0800, Leo Yan escreveu: > > > > On Thu, Aug 27, 2020 at 02:53:54PM -0600, Mathieu Poirier wrote: > > > > > On Wed, Aug 19, 2020 at 04:47:50PM +0800, Leo Yan wrote: > > > > > I've see this patch before... I thought it had been merged - what happened? > > > > > This patch before has been sent by Al to CoreSight mailing list but has > > > > not sent to LKML, this is why I resent it to LKML in case it's missed. > > > > So, was it Acked on the CoreSight mailing list? Are we missing any > > > Acked-by or Reviewed-by for this 1/2 patch as we got for 2/2? > > > The CoreSight mailing list has some discussion for this patch set, > > when respin this patch set, I confirmed we don't miss any 'Acked' or > > 'Reviewed' tags. > > So, this patch was included in today's > perf-tools-fixes-for-v5.9-2020-09-01 signed tag sent to Linus in a pull > req, please update your perf/urgent branch, and go on from there, I'll > merge that into my perf/core branch as soon as Linus merges it. Thanks for the info! Leo
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index c283223fb31f..a2a369e2fbb6 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -1344,8 +1344,15 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm, attr.sample_type &= ~(u64)PERF_SAMPLE_ADDR; } - if (etm->synth_opts.last_branch) + if (etm->synth_opts.last_branch) { attr.sample_type |= PERF_SAMPLE_BRANCH_STACK; + /* + * We don't use the hardware index, but the sample generation + * code uses the new format branch_stack with this field, + * so the event attributes must indicate that it's present. + */ + attr.branch_sample_type |= PERF_SAMPLE_BRANCH_HW_INDEX; + } if (etm->synth_opts.instructions) { attr.config = PERF_COUNT_HW_INSTRUCTIONS;