From: Viresh Kumar <viresh.kumar@linaro.org>
To: Ionela Voinescu <ionela.voinescu@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
Vincent Guittot <vincent.guittot@linaro.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: topology: Avoid the static_branch_{enable|disable} dance
Date: Thu, 10 Dec 2020 18:05:27 +0530 [thread overview]
Message-ID: <20201210123527.ccl4k56hl5mc3jag@vireshk-i7> (raw)
In-Reply-To: <20201210110906.GA5300@arm.com>
On 10-12-20, 11:09, Ionela Voinescu wrote:
> On Thursday 10 Dec 2020 at 15:12:25 (+0530), Viresh Kumar wrote:
> > Avoid the static_branch_enable() and static_branch_disable() dance by
> > redoing the code in a different way. We will be fully invariant here
> > only if amu_fie_cpus is set with all present CPUs, use that instead of
> > yet another call to topology_scale_freq_invariant().
> >
> > This also avoids running rest of the routine if we enabled the static
> > branch, followed by a disable.
> >
> > Also make the first call to topology_scale_freq_invariant() just when we
> > need it, instead of at the top of the routine. This makes it further
> > clear on why we need it, i.e. just around enabling AMUs use here.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > arch/arm64/kernel/topology.c | 21 +++++++++------------
> > 1 file changed, 9 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> > index 7f7d8de325b6..6dedc6ee91cf 100644
> > --- a/arch/arm64/kernel/topology.c
> > +++ b/arch/arm64/kernel/topology.c
> > @@ -221,7 +221,7 @@ static DEFINE_STATIC_KEY_FALSE(amu_fie_key);
> >
> > static int __init init_amu_fie(void)
> > {
> > - bool invariance_status = topology_scale_freq_invariant();
> > + bool invariance_status;
> > cpumask_var_t valid_cpus;
> > int ret = 0;
> > int cpu;
> > @@ -255,18 +255,15 @@ static int __init init_amu_fie(void)
> > cpumask_equal(valid_cpus, cpu_present_mask))
> > cpumask_copy(amu_fie_cpus, cpu_present_mask);
> >
> > - if (!cpumask_empty(amu_fie_cpus)) {
> > - pr_info("CPUs[%*pbl]: counters will be used for FIE.",
> > - cpumask_pr_args(amu_fie_cpus));
> > - static_branch_enable(&amu_fie_key);
> > - }
>
> This check verifies if there are *any* CPUs for which AMUs can be used for
> FIE (!empty mask) -> enable static key.
>
> > + /* Disallow partial use of counters for frequency invariance */
> > + if (!cpumask_equal(amu_fie_cpus, cpu_present_mask))
> > + goto free_valid_mask;
> >
>
> The replacement verifies if *all* present CPUs support AMUs for FIE and
> only then it enables the static key.
>
> > - /*
> > - * If the system is not fully invariant after AMU init, disable
> > - * partial use of counters for frequency invariance.
> > - */
> > - if (!topology_scale_freq_invariant())
I mis-read something here, as shared in the other thread, so yeah I
need to think again about this patch.
> > - static_branch_disable(&amu_fie_key);
> > + pr_info("CPUs[%*pbl]: counters will be used for FIE.",
> > + cpumask_pr_args(amu_fie_cpus));
> > +
> > + invariance_status = topology_scale_freq_invariant();
> > + static_branch_enable(&amu_fie_key);
> >
> > /*
> > * Task scheduler behavior depends on frequency invariance support,
> > --
> > 2.25.0.rc1.19.g042ed3e048af
> >
--
viresh
next prev parent reply other threads:[~2020-12-10 12:36 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-10 7:18 [PATCH] arm64: topology: Cleanup init_amu_fie() a bit Viresh Kumar
2020-12-10 9:42 ` [PATCH] arm64: topology: Avoid the static_branch_{enable|disable} dance Viresh Kumar
2020-12-10 11:09 ` Ionela Voinescu
2020-12-10 12:35 ` Viresh Kumar [this message]
2020-12-10 10:34 ` [PATCH] arm64: topology: Cleanup init_amu_fie() a bit Viresh Kumar
2020-12-10 10:38 ` Ionela Voinescu
2020-12-10 10:55 ` Viresh Kumar
2020-12-10 11:29 ` Ionela Voinescu
2020-12-10 12:34 ` Viresh Kumar
2020-12-10 13:26 ` Ionela Voinescu
2020-12-11 11:05 ` Viresh Kumar
2020-12-14 16:14 ` Ionela Voinescu
2020-12-10 13:22 ` Ionela Voinescu
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=20201210123527.ccl4k56hl5mc3jag@vireshk-i7 \
--to=viresh.kumar@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=ionela.voinescu@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vincent.guittot@linaro.org \
--cc=will@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).