[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJZ5v0iv22rCJ5uw-sguk6hgMhYn8EdXooHiMaD1pkR_d+zNnA@mail.gmail.com>
Date: Wed, 17 Feb 2021 15:18:20 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Giovanni Gherdovich <ggherdovich@...e.cz>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Linux PM <linux-pm@...r.kernel.org>,
Michael Larabel <Michael@...ronix.com>,
LKML <linux-kernel@...r.kernel.org>,
Linux ACPI <linux-acpi@...r.kernel.org>,
Peter Zijlstra <peterz@...radead.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Viresh Kumar <viresh.kumar@...aro.org>,
Mel Gorman <mgorman@...hsingularity.net>,
Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>
Subject: Re: [RFT][PATCH v1] cpufreq: ACPI: Set cpuinfo.max_freq directly if
max boost is known
On Wed, Feb 17, 2021 at 11:46 AM Giovanni Gherdovich
<ggherdovich@...e.cz> wrote:
>
> On Mon, 2021-02-15 at 20:24 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > Commit 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover
> > boost frequencies") attempted to address a performance issue involving
> > acpi-cpufreq, the schedutil governor and scale-invariance on x86 by
> > extending the frequency tables created by acpi-cpufreq to cover the
> > entire range of "turbo" (or "boost") frequencies, but that caused
> > frequencies reported via /proc/cpuinfo and the scaling_cur_freq
> > attribute in sysfs to change which may confuse users and monitoring
> > tools.
> >
> > For this reason, revert the part of commit 3c55e94c0ade adding the
> > extra entry to the frequency table and use the observation that
> > in principle cpuinfo.max_freq need not be equal to the maximum
> > frequency listed in the frequency table for the given policy.
> >
> > Namely, modify cpufreq_frequency_table_cpuinfo() to allow cpufreq
> > drivers to set their own cpuinfo.max_freq above that frequency and
> > change acpi-cpufreq to set cpuinfo.max_freq to the maximum boost
> > frequency found via CPPC.
> >
> > This should be sufficient to let all of the cpufreq subsystem know
> > the real maximum frequency of the CPU without changing frequency
> > reporting.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=211305
> > Fixes: 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover boost frequencies")
> > Reported-by: Matt McDonald <gardotd426@...il.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > ---
> >
> > Michael, Giovanni,
> >
> > The fix for the EPYC performance regression that was merged into 5.11 introduced
> > an undesirable side-effect by distorting the CPU frequency reporting via
> > /proc/cpuinfo and scaling_cur_freq (see the BZ link above for details).
> >
> > The patch below is reported to address this problem and it should still allow
> > schedutil to achieve desirable performance, because it simply sets
> > cpuinfo.max_freq without extending the frequency table of the CPU.
> >
> > Please test this one and let me know if it adversely affects performance.
> >
> > Thanks!
>
> Hello Rafael,
>
> more extended testing confirms the initial feeling; performance with this
> patch is mostly identical to vanilla v5.11.
Thank you!
> Tbench shows an improvement.
Interesting.
> Thanks for the fix!
YW
> Tested-by: Giovanni Gherdovich <ggherdovich@...e.cz>
>
> Results follow. The machine has two sockets with an AMD EPYC 7742 each.
> The governor is always schedutil.
>
>
> Ratios of time, lower is better:
> v5.11 v5.11
> vanilla patch
> - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> NASA Parallel Benchmarks w/ MPI 1.00 0.96
> NASA Parallel Benchmarks w/ OpenMP 1.00 ~
> dbench on XFS 1.00 ~
> Linux kernel compilation 1.00 ~
> git unit test suite 1.00 ~
>
>
> Ratio of throughput, higher is better:
> v5.11 v5.11
> vanilla patch
> - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> tbench on localhost 1.00 1.09
>
>
> Tilde (~): no change wrt baseline.
Thanks again!
It would be good to hear from Michael too, but this is already
sufficient for me to queue up the patch for 5.12-rc.
Cheers!
Powered by blists - more mailing lists