All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Yicong Yang <yangyicong@huawei.com>
Cc: will@kernel.org, linux-arm-kernel@lists.infradead.org,
	james.clark@linaro.org, robh@kernel.org,
	anshuman.khandual@arm.com, jonathan.cameron@huawei.com,
	hejunhao3@huawei.com, linuxarm@huawei.com,
	prime.zeng@hisilicon.com, xuwei5@huawei.com,
	wangyushan12@huawei.com, yangyicong@hisilicon.com
Subject: Re: [PATCH 2/2] perf: arm_pmuv3: Don't use PMCCNTR_EL0 on SMT cores
Date: Tue, 12 Aug 2025 11:33:14 +0100	[thread overview]
Message-ID: <aJsYah_DGPIQ5v3-@J2N7QTR9R3> (raw)
In-Reply-To: <20250812080830.20796-3-yangyicong@huawei.com>

On Tue, Aug 12, 2025 at 04:08:30PM +0800, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> CPU_CYCLES is expected to count the logical CPU (PE) clock. Currently it's
> preferred to use PMCCNTR_EL0 for counting CPU_CYCLES, but it'll count
> processor clock rather than the PE clock (ARM DDI0487 L.b D13.1.3) if
> one of the SMT siblings is not idle on a multi-threaded implementation.
>
> So don't use it on SMT cores.

This is rather unfortunate.

When does this actually matter?

Per ARM DDI 0487 L.b, page D14-6918:

| If FEAT_PMUv3p9 is implemented, then CPU_CYCLES does not increment
| when the clocks are stopped by WFI and WFE instructions. Otherwise, it
| is CONSTRAINED UNPREDICTABLE whether or not CPU_CYCLES continues to
| increment when the clocks are stopped by WFI and WFE instructions.

... so prior to FEAT_PMUv3p9, no-one could rely on the difference
anyway.

> When counting cycles on SMT CPU 2-3 and CPU 3 is idle, without this
> patch we'll get:
> [root@client1 tmp]# perf stat -e cycles -A -C 2-3 -- stress-ng -c 1
> --taskset 2 --timeout 1
> [...]
>  Performance counter stats for 'CPU(s) 2-3':
> 
> CPU2           2880457316      cycles
> CPU3           2880459810      cycles
>        1.254688470 seconds time elapsed
> 
> With this patch the idle state of CPU3 is observed as expected:
> [root@client1 ~]#  perf stat -e cycles -A -C 2-3 -- stress-ng -c 1
> --taskset 2 --timeout 1
> [...]
>  Performance counter stats for 'CPU(s) 2-3':
> 
> CPU2           2558580492      cycles
> CPU3               305749      cycles
>        1.113626410 seconds time elapsed
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
>  drivers/perf/arm_pmuv3.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 95c899d07df5..ed3149632b71 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -1002,6 +1002,15 @@ static bool armv8pmu_can_use_pmccntr(struct pmu_hw_events *cpuc,
>  	if (has_branch_stack(event))
>  		return false;
>  
> +	/*
> +	 * The PMCCNTR_EL0 increments from the processor clock rather than
> +	 * the PE clock (ARM DDI0487 L.b D13.1.3) which means it'll continue
> +	 * counting on a WFI PE if one of its SMT silbing is not idle on a
> +	 * multi-threaded implementation. So don't use it on SMT cores.
> +	 */
> +	if (cpumask_weight(topology_sibling_cpumask(smp_processor_id())) > 1)
> +		return false;

This effectively forbids use of PMCCNTR_EL0 for any events.

Is there any existing event that it is useful for?

Mark.

> +
>  	return true;
>  }
>  
> -- 
> 2.24.0
> 


  parent reply	other threads:[~2025-08-12 16:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-12  8:08 [PATCH 0/2] perf: arm_pmuv3: Don't use PMCCNTR_EL0 on SMT cores Yicong Yang
2025-08-12  8:08 ` [PATCH 1/2] perf: arm_pmuv3: Factor out PMCCNTR_EL0 use conditions Yicong Yang
2025-08-12 10:02   ` James Clark
2025-08-12 10:25   ` Mark Rutland
2025-08-12  8:08 ` [PATCH 2/2] perf: arm_pmuv3: Don't use PMCCNTR_EL0 on SMT cores Yicong Yang
2025-08-12 10:00   ` James Clark
2025-08-12 10:14     ` Yicong Yang
2025-08-12 10:22       ` Mark Rutland
2025-08-13  8:17         ` Yicong Yang
2025-08-12 10:31       ` James Clark
2025-08-13  8:32         ` Yicong Yang
2025-08-12 10:33   ` Mark Rutland [this message]
2025-08-13  8:03     ` Yicong Yang

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=aJsYah_DGPIQ5v3-@J2N7QTR9R3 \
    --to=mark.rutland@arm.com \
    --cc=anshuman.khandual@arm.com \
    --cc=hejunhao3@huawei.com \
    --cc=james.clark@linaro.org \
    --cc=jonathan.cameron@huawei.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linuxarm@huawei.com \
    --cc=prime.zeng@hisilicon.com \
    --cc=robh@kernel.org \
    --cc=wangyushan12@huawei.com \
    --cc=will@kernel.org \
    --cc=xuwei5@huawei.com \
    --cc=yangyicong@hisilicon.com \
    --cc=yangyicong@huawei.com \
    /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: link
Be 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.