linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] Patch "perf tools: Fix pattern matching for same substring in different pmu type" broken
@ 2021-07-19 14:13 John Garry
  2021-07-19 20:01 ` Arnaldo Carvalho de Melo
  2021-07-20  5:56 ` Jin, Yao
  0 siblings, 2 replies; 3+ messages in thread
From: John Garry @ 2021-07-19 14:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa, Kan Liang, Jin Yao
  Cc: linux-kernel, linux-perf-users, Zhangshaokun, liuqi (BA)

Hi guys,

The named patch has broken PMU alias matching on my arm64 system.

Specifically it is broken for when multiple tokens are used in the 
alias. For example, alias "hisi_sccl,l3c" would previously match for PMU 
"hisi_sccl3_l3c7", but that no longer works.

In my example, in looking at the code, the callchain 
pmu_uncore_alias_match("hisi_sccl,l3c", "hisi_sccl3_l3c7") -> 
per_pmu__valid_suffix("hisi_sccl3_l3c7", "hisi_sccl") fails in the 
following check:

static bool perf_pmu__valid_suffix(char *pmu_name, char *tok)
{
	char *p;
	...
	p = pmu_name + strlen(tok);
	...
	if (*p != '_') //here
		return false;
}

This check assumes the first token must be followed by a '_', but it is 
possibly a numeric.

Please let me know how this should work. Previously it would match on 
the tokens, ignoring numerics and '_'.

As an aside, I'll look at why our testcases don't cover this scenario 
and look to add a test if necessary.

Thanks,
john

Ps, please cc linux-perf-users@vger.kernel.org as in the MAINTAINERS 
file in future, as not all subscribe to the open kernel list (and so 
cannot easily reply directly).


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] Patch "perf tools: Fix pattern matching for same substring in different pmu type" broken
  2021-07-19 14:13 [bug report] Patch "perf tools: Fix pattern matching for same substring in different pmu type" broken John Garry
@ 2021-07-19 20:01 ` Arnaldo Carvalho de Melo
  2021-07-20  5:56 ` Jin, Yao
  1 sibling, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-19 20:01 UTC (permalink / raw)
  To: Jin Yao, John Garry
  Cc: Jiri Olsa, Kan Liang, Jin Yao, linux-kernel, linux-perf-users,
	Zhangshaokun, liuqi (BA)

Em Mon, Jul 19, 2021 at 03:13:06PM +0100, John Garry escreveu:
> Hi guys,
> 
> The named patch has broken PMU alias matching on my arm64 system.

Hi Jin, can you please address this report? Otherwise I'll have to
revert the patch in my next pull req to Linus :-\

- Arnaldo
 
> Specifically it is broken for when multiple tokens are used in the alias.
> For example, alias "hisi_sccl,l3c" would previously match for PMU
> "hisi_sccl3_l3c7", but that no longer works.
> 
> In my example, in looking at the code, the callchain
> pmu_uncore_alias_match("hisi_sccl,l3c", "hisi_sccl3_l3c7") ->
> per_pmu__valid_suffix("hisi_sccl3_l3c7", "hisi_sccl") fails in the following
> check:
> 
> static bool perf_pmu__valid_suffix(char *pmu_name, char *tok)
> {
> 	char *p;
> 	...
> 	p = pmu_name + strlen(tok);
> 	...
> 	if (*p != '_') //here
> 		return false;
> }
> 
> This check assumes the first token must be followed by a '_', but it is
> possibly a numeric.
> 
> Please let me know how this should work. Previously it would match on the
> tokens, ignoring numerics and '_'.
> 
> As an aside, I'll look at why our testcases don't cover this scenario and
> look to add a test if necessary.
> 
> Thanks,
> john
> 
> Ps, please cc linux-perf-users@vger.kernel.org as in the MAINTAINERS file in
> future, as not all subscribe to the open kernel list (and so cannot easily
> reply directly).
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] Patch "perf tools: Fix pattern matching for same substring in different pmu type" broken
  2021-07-19 14:13 [bug report] Patch "perf tools: Fix pattern matching for same substring in different pmu type" broken John Garry
  2021-07-19 20:01 ` Arnaldo Carvalho de Melo
@ 2021-07-20  5:56 ` Jin, Yao
  1 sibling, 0 replies; 3+ messages in thread
From: Jin, Yao @ 2021-07-20  5:56 UTC (permalink / raw)
  To: John Garry, Arnaldo Carvalho de Melo, Jiri Olsa, Kan Liang
  Cc: linux-kernel, linux-perf-users, Zhangshaokun, liuqi (BA),
	Andi Kleen, Jin, Yao

Hi John, Hi Arnaldo,

On 7/19/2021 10:13 PM, John Garry wrote:
> Hi guys,
> 
> The named patch has broken PMU alias matching on my arm64 system.
> 
> Specifically it is broken for when multiple tokens are used in the alias. For example, alias 
> "hisi_sccl,l3c" would previously match for PMU "hisi_sccl3_l3c7", but that no longer works.
> 
> In my example, in looking at the code, the callchain pmu_uncore_alias_match("hisi_sccl,l3c", 
> "hisi_sccl3_l3c7") -> per_pmu__valid_suffix("hisi_sccl3_l3c7", "hisi_sccl") fails in the following 
> check:
> 
> static bool perf_pmu__valid_suffix(char *pmu_name, char *tok)
> {
>      char *p;
>      ...
>      p = pmu_name + strlen(tok);
>      ...
>      if (*p != '_') //here
>          return false;
> }
> 
> This check assumes the first token must be followed by a '_', but it is possibly a numeric.
> 

It looks that the PMU alias format on arm64 has big difference than the format on x86. My new idea 
is we create a x86 specific perf_pmu__valid_suffix, and for other arch, the weak function always 
returns true. That will not change original behavior.

What do you think?

> Please let me know how this should work. Previously it would match on the tokens, ignoring numerics 
> and '_'.
> 
> As an aside, I'll look at why our testcases don't cover this scenario and look to add a test if 
> necessary.
> 
> Thanks,
> john
> 
> Ps, please cc linux-perf-users@vger.kernel.org as in the MAINTAINERS file in future, as not all 
> subscribe to the open kernel list (and so cannot easily reply directly).
> 

Sorry about that! I will remember that, cc linux-perf-users@vger.kernel.org in next patch.

Thanks
Jin Yao

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-07-20  5:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-19 14:13 [bug report] Patch "perf tools: Fix pattern matching for same substring in different pmu type" broken John Garry
2021-07-19 20:01 ` Arnaldo Carvalho de Melo
2021-07-20  5:56 ` Jin, Yao

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox