2024-02-29 16:28:32

by Vanshidhar Konda

Subject: [PATCH v1 2/3] arm64: idle: Cache AMU counters before entering idle

AMU counters do not increment while a CPU is in idle. Saving the value
of the core and constant counters prior to invoking WFI allows FIE to
compute the frequency of a CPU that is idle.

Signed-off-by: Vanshidhar Konda <[email protected]>
---
arch/arm64/kernel/idle.c | 10 ++++++++++
arch/arm64/kernel/topology.c | 14 ++++++++------
2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/idle.c b/arch/arm64/kernel/idle.c
index 05cfb347ec26..5ed2e57188a8 100644
--- a/arch/arm64/kernel/idle.c
+++ b/arch/arm64/kernel/idle.c
@@ -26,6 +26,16 @@ void __cpuidle cpu_do_idle(void)

arm_cpuidle_save_irq_context(&context);

+#ifdef CONFIG_ARM64_AMU_EXTN
+ /* Update the AMU counters before entering WFI. The cached AMU counter
+ * value is used to determine CPU frequency while the CPU is idle
+ * without needing to wake up the CPU.
+ */
+
+ if (cpu_has_amu_feat(smp_processor_id()))
+ update_freq_counters_refs();
+#endif
+
dsb(sy);
wfi();

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index db8d14525cf4..8905eb0c681f 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -240,13 +240,15 @@ unsigned int arch_freq_get_on_cpu(int cpu)
} while (read_seqcount_retry(&cpu_sample->seq, seq));

/*
- * Bail on invalid count and when the last update was too long ago,
- * which covers idle and NOHZ full CPUs.
+ * Bail on invalid count and when the last update was too long ago.
+ * This covers idle, NOHZ full and isolated CPUs.
+ *
+ * Idle CPUs don't need to be measured because AMU counters stop
+ * incrementing during WFI/WFE.
*/
- if (!delta_const_cnt || ((jiffies - last) > MAX_SAMPLE_AGE)) {
- if (!(housekeeping_cpu(cpu, HK_TYPE_TICK) && idle_cpu(cpu)))
- goto fallback;
- }
+ if (!delta_const_cnt ||
+ ((jiffies - last) > MAX_SAMPLE_AGE && !idle_cpu(cpu)))
+ goto fallback;

/*
* CPU frequency = reference perf (in Hz) * (/\ delivered) / (/\ reference)
--
2.43.1

2024-03-07 03:18:17

by Huisong Li

Subject: Re: [PATCH v1 2/3] arm64: idle: Cache AMU counters before entering idle

在 2024/3/1 0:25, Vanshidhar Konda 写道:
> AMU counters do not increment while a CPU is in idle. Saving the value
> of the core and constant counters prior to invoking WFI allows FIE to
> compute the frequency of a CPU that is idle.
>
> Signed-off-by: Vanshidhar Konda <[email protected]>
> ---
> arch/arm64/kernel/idle.c | 10 ++++++++++
> arch/arm64/kernel/topology.c | 14 ++++++++------
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/kernel/idle.c b/arch/arm64/kernel/idle.c
> index 05cfb347ec26..5ed2e57188a8 100644
> --- a/arch/arm64/kernel/idle.c
> +++ b/arch/arm64/kernel/idle.c
> @@ -26,6 +26,16 @@ void __cpuidle cpu_do_idle(void)
>
> arm_cpuidle_save_irq_context(&context);
>
> +#ifdef CONFIG_ARM64_AMU_EXTN
> + /* Update the AMU counters before entering WFI. The cached AMU counter
> + * value is used to determine CPU frequency while the CPU is idle
> + * without needing to wake up the CPU.
> + */
> +
> + if (cpu_has_amu_feat(smp_processor_id()))
> + update_freq_counters_refs();
> +#endif
The below point I has mentioned in [1].
This is just for the WFI state.
What about other deeper idle states, like retention and power down?
The path to enter idle state is different for them. We should do this
for all idle states.

> +
> dsb(sy);
> wfi();
>
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index db8d14525cf4..8905eb0c681f 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -240,13 +240,15 @@ unsigned int arch_freq_get_on_cpu(int cpu)
> } while (read_seqcount_retry(&cpu_sample->seq, seq));
>
> /*
> - * Bail on invalid count and when the last update was too long ago,
> - * which covers idle and NOHZ full CPUs.
> + * Bail on invalid count and when the last update was too long ago.
> + * This covers idle, NOHZ full and isolated CPUs.
> + *
> + * Idle CPUs don't need to be measured because AMU counters stop
> + * incrementing during WFI/WFE.
> */
> - if (!delta_const_cnt || ((jiffies - last) > MAX_SAMPLE_AGE)) {
> - if (!(housekeeping_cpu(cpu, HK_TYPE_TICK) && idle_cpu(cpu)))
> - goto fallback;
> - }
> + if (!delta_const_cnt ||
> + ((jiffies - last) > MAX_SAMPLE_AGE && !idle_cpu(cpu)))
> + goto fallback;
>
> /*
> * CPU frequency = reference perf (in Hz) * (/\ delivered) / (/\ reference)
[1]
https://lore.kernel.org/linux-arm-kernel/[email protected]/

2024-03-11 18:27:49

by Vanshidhar Konda

Subject: Re: Re: [PATCH v1 2/3] arm64: idle: Cache AMU counters before entering idle

On Thu, Mar 07, 2024 at 11:17:26AM +0800, lihuisong (C) wrote:
>
>在 2024/3/1 0:25, Vanshidhar Konda 写道:
>>AMU counters do not increment while a CPU is in idle. Saving the value
>>of the core and constant counters prior to invoking WFI allows FIE to
>>compute the frequency of a CPU that is idle.
>>
>>Signed-off-by: Vanshidhar Konda <[email protected]>
>>---
>> arch/arm64/kernel/idle.c | 10 ++++++++++
>> arch/arm64/kernel/topology.c | 14 ++++++++------
>> 2 files changed, 18 insertions(+), 6 deletions(-)
>>
>>diff --git a/arch/arm64/kernel/idle.c b/arch/arm64/kernel/idle.c
>>index 05cfb347ec26..5ed2e57188a8 100644
>>--- a/arch/arm64/kernel/idle.c
>>+++ b/arch/arm64/kernel/idle.c
>>@@ -26,6 +26,16 @@ void __cpuidle cpu_do_idle(void)
>> arm_cpuidle_save_irq_context(&context);
>>+#ifdef CONFIG_ARM64_AMU_EXTN
>>+ /* Update the AMU counters before entering WFI. The cached AMU counter
>>+ * value is used to determine CPU frequency while the CPU is idle
>>+ * without needing to wake up the CPU.
>>+ */
>>+
>>+ if (cpu_has_amu_feat(smp_processor_id()))
>>+ update_freq_counters_refs();
>>+#endif
>The below point I has mentioned in [1].
>This is just for the WFI state.
>What about other deeper idle states, like retention and power down?
>The path to enter idle state is different for them. We should do this
>for all idle states.
>

Yes. That makes sense. I'll account for them in the next version of the
patch. I'll work on the next version of the patch based on the updated
patch from @Beata.

Thanks,
Vanshi

>>+
>> dsb(sy);
>> wfi();
>>diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
>>index db8d14525cf4..8905eb0c681f 100644
>>--- a/arch/arm64/kernel/topology.c
>>+++ b/arch/arm64/kernel/topology.c
>>@@ -240,13 +240,15 @@ unsigned int arch_freq_get_on_cpu(int cpu)
>> } while (read_seqcount_retry(&cpu_sample->seq, seq));
>> /*
>>- * Bail on invalid count and when the last update was too long ago,
>>- * which covers idle and NOHZ full CPUs.
>>+ * Bail on invalid count and when the last update was too long ago.
>>+ * This covers idle, NOHZ full and isolated CPUs.
>>+ *
>>+ * Idle CPUs don't need to be measured because AMU counters stop
>>+ * incrementing during WFI/WFE.
>> */
>>- if (!delta_const_cnt || ((jiffies - last) > MAX_SAMPLE_AGE)) {
>>- if (!(housekeeping_cpu(cpu, HK_TYPE_TICK) && idle_cpu(cpu)))
>>- goto fallback;
>>- }
>>+ if (!delta_const_cnt ||
>>+ ((jiffies - last) > MAX_SAMPLE_AGE && !idle_cpu(cpu)))
>>+ goto fallback;
>> /*
>> * CPU frequency = reference perf (in Hz) * (/\ delivered) / (/\ reference)
>[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
>

2024-03-12 08:45:50

by Beata Michalska

Subject: Re: Re: [PATCH v1 2/3] arm64: idle: Cache AMU counters before entering idle

On Mon, Mar 11, 2024 at 11:27:27AM -0700, Vanshidhar Konda wrote:
> On Thu, Mar 07, 2024 at 11:17:26AM +0800, lihuisong (C) wrote:
> >
> > 在 2024/3/1 0:25, Vanshidhar Konda 写道:
> > > AMU counters do not increment while a CPU is in idle. Saving the value
> > > of the core and constant counters prior to invoking WFI allows FIE to
> > > compute the frequency of a CPU that is idle.
> > >
> > > Signed-off-by: Vanshidhar Konda <[email protected]>
> > > ---
> > > arch/arm64/kernel/idle.c | 10 ++++++++++
> > > arch/arm64/kernel/topology.c | 14 ++++++++------
> > > 2 files changed, 18 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/idle.c b/arch/arm64/kernel/idle.c
> > > index 05cfb347ec26..5ed2e57188a8 100644
> > > --- a/arch/arm64/kernel/idle.c
> > > +++ b/arch/arm64/kernel/idle.c
> > > @@ -26,6 +26,16 @@ void __cpuidle cpu_do_idle(void)
> > > arm_cpuidle_save_irq_context(&context);
> > > +#ifdef CONFIG_ARM64_AMU_EXTN
> > > + /* Update the AMU counters before entering WFI. The cached AMU counter
> > > + * value is used to determine CPU frequency while the CPU is idle
> > > + * without needing to wake up the CPU.
> > > + */
> > > +
> > > + if (cpu_has_amu_feat(smp_processor_id()))
> > > + update_freq_counters_refs();
> > > +#endif
> > The below point I has mentioned in [1].
> > This is just for the WFI state.
> > What about other deeper idle states, like retention and power down?
> > The path to enter idle state is different for them. We should do this
> > for all idle states.
> >
>
> Yes. That makes sense. I'll account for them in the next version of the
> patch. I'll work on the next version of the patch based on the updated
> patch from @Beata.
>
This should now be covered by [1]

---
[1]https://lore.kernel.org/all/[email protected]/

---
BR
Beata

> Thanks,
> Vanshi
>
> > > +
> > > dsb(sy);
> > > wfi();
> > > diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> > > index db8d14525cf4..8905eb0c681f 100644
> > > --- a/arch/arm64/kernel/topology.c
> > > +++ b/arch/arm64/kernel/topology.c
> > > @@ -240,13 +240,15 @@ unsigned int arch_freq_get_on_cpu(int cpu)
> > > } while (read_seqcount_retry(&cpu_sample->seq, seq));
> > > /*
> > > - * Bail on invalid count and when the last update was too long ago,
> > > - * which covers idle and NOHZ full CPUs.
> > > + * Bail on invalid count and when the last update was too long ago.
> > > + * This covers idle, NOHZ full and isolated CPUs.
> > > + *
> > > + * Idle CPUs don't need to be measured because AMU counters stop
> > > + * incrementing during WFI/WFE.
> > > */
> > > - if (!delta_const_cnt || ((jiffies - last) > MAX_SAMPLE_AGE)) {
> > > - if (!(housekeeping_cpu(cpu, HK_TYPE_TICK) && idle_cpu(cpu)))
> > > - goto fallback;
> > > - }
> > > + if (!delta_const_cnt ||
> > > + ((jiffies - last) > MAX_SAMPLE_AGE && !idle_cpu(cpu)))
> > > + goto fallback;
> > > /*
> > > * CPU frequency = reference perf (in Hz) * (/\ delivered) / (/\ reference)
> > [1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
> >