All of lore.kernel.org
 help / color / mirror / Atom feed
From: Beata Michalska <beata.michalska@arm.com>
To: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, ionela.voinescu@arm.com,
	sudeep.holla@arm.com, will@kernel.org, catalin.marinas@arm.com,
	vincent.guittot@linaro.org, sumitg@nvidia.com,
	yang@os.amperecomputing.com, lihuisong@huawei.com
Subject: Re: [PATCH v3 0/3] Add support for AArch64 AMUv1-based arch_freq_get_on_cpu
Date: Wed, 3 Apr 2024 23:34:49 +0200	[thread overview]
Message-ID: <Zg3LeYvOefchf1N3@arm.com> (raw)
In-Reply-To: <5bdlm4kzni6x2bdy7kmmomf7cmyohjhr4nr7v2mb2pchuhkulj@moakmpptnbg5>

On Mon, Mar 25, 2024 at 09:10:26AM -0700, Vanshidhar Konda wrote:
> On Tue, Mar 12, 2024 at 08:34:28AM +0000, Beata Michalska wrote:
> > Introducing arm64 specific version of arch_freq_get_on_cpu, cashing on
> > existing implementation for FIE and AMUv1 support: the frequency scale
> > factor, updated on each sched tick, serves as a base for retrieving
> > the frequency for a given CPU, representing an average frequency
> > reported between the ticks - thus its accuracy is limited.
> > 
> > The changes have been rather lightly (due to some limitations) tested on
> > an FVP model.
> > 
> 
> I tested these changes on an Ampere system. The results from reading
> scaling_cur_freq look reasonable in the majority of cases I tested. I
> only saw some unexpected behavior with cores that were configured for
> no_hz full.
> 
> I observed the unexplained behavior when I tested as follows:
> 1. Run stress on all cores
>    stress-ng --cpu 186 --timeout 10m --metrics-brief
> 2. Observe scaling_cur_freq and cpuinfo_cur_freq for all cores
>    scaling_cur_freq values were within a few % of cpuinfo_cur_freq
> 3. Kill stress test
> 4. Observe scaling_cur_freq and cpuinfo_cur_freq for all cores
>    scaling_cur_freq values were within a few % of cpuinfo_cur_freq for
>    most cores except the ones configured with no_hz full.
> 
> no_hz full = 122-127
> core   scaling_cur_freq  cpuinfo_cur_freq
> [122]: 2997070           1000000
> [123]: 2997070           1000000
> [124]: 3000038           1000000
> [125]: 2997070           1000000
> [126]: 2997070           1000000
> [127]: 2997070           1000000
> 
> These values were reflected for multiple seconds. I suspect the cores
> entered WFI and there was no update to the scale while those cores were
> idle.
>
Right, so the problem is with updating the counters upon entering idle, which at
this point is being done for all CPUs, and it should exclude the full dynticks
ones - otherwise it leads to such bad readings. So for nohz_full cores cpufreq
driver will have to take care of getting the current frequency.

Will be sending a fix for that.

Thank you very much for testing - appreciate that!

---
BR
Beata
> Thanks,
> Vanshi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Beata Michalska <beata.michalska@arm.com>
To: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, ionela.voinescu@arm.com,
	sudeep.holla@arm.com, will@kernel.org, catalin.marinas@arm.com,
	vincent.guittot@linaro.org, sumitg@nvidia.com,
	yang@os.amperecomputing.com, lihuisong@huawei.com
Subject: Re: [PATCH v3 0/3] Add support for AArch64 AMUv1-based arch_freq_get_on_cpu
Date: Wed, 3 Apr 2024 23:34:49 +0200	[thread overview]
Message-ID: <Zg3LeYvOefchf1N3@arm.com> (raw)
In-Reply-To: <5bdlm4kzni6x2bdy7kmmomf7cmyohjhr4nr7v2mb2pchuhkulj@moakmpptnbg5>

On Mon, Mar 25, 2024 at 09:10:26AM -0700, Vanshidhar Konda wrote:
> On Tue, Mar 12, 2024 at 08:34:28AM +0000, Beata Michalska wrote:
> > Introducing arm64 specific version of arch_freq_get_on_cpu, cashing on
> > existing implementation for FIE and AMUv1 support: the frequency scale
> > factor, updated on each sched tick, serves as a base for retrieving
> > the frequency for a given CPU, representing an average frequency
> > reported between the ticks - thus its accuracy is limited.
> > 
> > The changes have been rather lightly (due to some limitations) tested on
> > an FVP model.
> > 
> 
> I tested these changes on an Ampere system. The results from reading
> scaling_cur_freq look reasonable in the majority of cases I tested. I
> only saw some unexpected behavior with cores that were configured for
> no_hz full.
> 
> I observed the unexplained behavior when I tested as follows:
> 1. Run stress on all cores
>    stress-ng --cpu 186 --timeout 10m --metrics-brief
> 2. Observe scaling_cur_freq and cpuinfo_cur_freq for all cores
>    scaling_cur_freq values were within a few % of cpuinfo_cur_freq
> 3. Kill stress test
> 4. Observe scaling_cur_freq and cpuinfo_cur_freq for all cores
>    scaling_cur_freq values were within a few % of cpuinfo_cur_freq for
>    most cores except the ones configured with no_hz full.
> 
> no_hz full = 122-127
> core   scaling_cur_freq  cpuinfo_cur_freq
> [122]: 2997070           1000000
> [123]: 2997070           1000000
> [124]: 3000038           1000000
> [125]: 2997070           1000000
> [126]: 2997070           1000000
> [127]: 2997070           1000000
> 
> These values were reflected for multiple seconds. I suspect the cores
> entered WFI and there was no update to the scale while those cores were
> idle.
>
Right, so the problem is with updating the counters upon entering idle, which at
this point is being done for all CPUs, and it should exclude the full dynticks
ones - otherwise it leads to such bad readings. So for nohz_full cores cpufreq
driver will have to take care of getting the current frequency.

Will be sending a fix for that.

Thank you very much for testing - appreciate that!

---
BR
Beata
> Thanks,
> Vanshi

  reply	other threads:[~2024-04-03 21:35 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-12  8:34 [PATCH v3 0/3] Add support for AArch64 AMUv1-based arch_freq_get_on_cpu Beata Michalska
2024-03-12  8:34 ` Beata Michalska
2024-03-12  8:34 ` [PATCH v3 1/3] arch_topology: init capacity_freq_ref to 0 Beata Michalska
2024-03-12  8:34   ` Beata Michalska
2024-03-12  8:34 ` [PATCH v3 2/3] arm64: Provide an AMU-based version of arch_freq_get_on_cpu Beata Michalska
2024-03-12  8:34   ` Beata Michalska
2024-03-13  2:12   ` Vanshidhar Konda
2024-03-13  2:12     ` Vanshidhar Konda
2024-03-13 21:47     ` Beata Michalska
2024-03-13 21:47       ` Beata Michalska
2024-03-13 12:20   ` Ionela Voinescu
2024-03-13 12:20     ` Ionela Voinescu
2024-03-13 23:46     ` Beata Michalska
2024-03-13 23:46       ` Beata Michalska
2024-03-18 15:01       ` Ionela Voinescu
2024-03-18 15:01         ` Ionela Voinescu
2024-03-20 16:43   ` Sumit Gupta
2024-03-20 16:43     ` Sumit Gupta
2024-04-03 21:28     ` Beata Michalska
2024-04-03 21:28       ` Beata Michalska
2024-03-12  8:34 ` [PATCH v3 3/3] arm64: Update AMU-based frequency scale factor on entering idle Beata Michalska
2024-03-12  8:34   ` Beata Michalska
2024-03-13 12:27 ` [PATCH v3 0/3] Add support for AArch64 AMUv1-based arch_freq_get_on_cpu Ionela Voinescu
2024-03-13 12:27   ` Ionela Voinescu
2024-03-13 23:49   ` Beata Michalska
2024-03-13 23:49     ` Beata Michalska
2024-03-20 16:52     ` Sumit Gupta
2024-03-20 16:52       ` Sumit Gupta
2024-04-03 21:30       ` Beata Michalska
2024-04-03 21:30         ` Beata Michalska
2024-03-25 16:10 ` Vanshidhar Konda
2024-03-25 16:10   ` Vanshidhar Konda
2024-04-03 21:34   ` Beata Michalska [this message]
2024-04-03 21:34     ` Beata Michalska

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=Zg3LeYvOefchf1N3@arm.com \
    --to=beata.michalska@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=ionela.voinescu@arm.com \
    --cc=lihuisong@huawei.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sudeep.holla@arm.com \
    --cc=sumitg@nvidia.com \
    --cc=vanshikonda@os.amperecomputing.com \
    --cc=vincent.guittot@linaro.org \
    --cc=will@kernel.org \
    --cc=yang@os.amperecomputing.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.