knl_set_max_freq_ratio() should return true on success and false
otherwise. If the last line of the function body is reached, it means no
appropriate value for turbo_freq was found: the setup is unsuccessful and
it should return false.
Fixes: 8bea0dfb4a82 ("x86, sched: Add support for frequency invariance on XEON_PHI_KNL/KNM")
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Giovanni Gherdovich <[email protected]>
---
arch/x86/kernel/smpboot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 2ef14772dc04..225a3c31297c 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1985,7 +1985,7 @@ static bool knl_set_max_freq_ratio(u64 *base_freq, u64 *turbo_freq,
i += 8;
} while (i < 64);
- return true;
+ return false;
}
static bool skx_set_max_freq_ratio(u64 *base_freq, u64 *turbo_freq, int size)
--
2.31.1
Once the microarchitecture is recognized (via x86_match_cpu()), a failure
in setting base_freq/turbo_freq should result in bailing out from frequency
invariance, not in trying the next microarchitecture. This is because the
call to core_set_max_freq_ratio() isn't guarded by x86_match_cpu(). The
call to core_set_max_freq_ratio() should happen if no more specific
microarch matched, but not in case of prior errors.
Initializing base_freq=0 and turbo_freq=0 gives a mean for later code to
check if setup failed.
Fixes: db441bd9f630 ("x86, sched: Move check for CPU type to caller function")
Signed-off-by: Giovanni Gherdovich <[email protected]>
---
arch/x86/kernel/smpboot.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 225a3c31297c..d0a692ea8294 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -2044,23 +2044,26 @@ static bool core_set_max_freq_ratio(u64 *base_freq, u64 *turbo_freq)
static bool intel_set_max_freq_ratio(void)
{
- u64 base_freq, turbo_freq;
+ u64 base_freq = 0, turbo_freq = 0;
u64 turbo_ratio;
if (slv_set_max_freq_ratio(&base_freq, &turbo_freq))
goto out;
- if (x86_match_cpu(has_glm_turbo_ratio_limits) &&
- skx_set_max_freq_ratio(&base_freq, &turbo_freq, 1))
+ if (x86_match_cpu(has_glm_turbo_ratio_limits)) {
+ skx_set_max_freq_ratio(&base_freq, &turbo_freq, 1);
goto out;
+ }
- if (x86_match_cpu(has_knl_turbo_ratio_limits) &&
- knl_set_max_freq_ratio(&base_freq, &turbo_freq, 1))
+ if (x86_match_cpu(has_knl_turbo_ratio_limits)) {
+ knl_set_max_freq_ratio(&base_freq, &turbo_freq, 1);
goto out;
+ }
- if (x86_match_cpu(has_skx_turbo_ratio_limits) &&
- skx_set_max_freq_ratio(&base_freq, &turbo_freq, 4))
+ if (x86_match_cpu(has_skx_turbo_ratio_limits)) {
+ skx_set_max_freq_ratio(&base_freq, &turbo_freq, 4);
goto out;
+ }
if (core_set_max_freq_ratio(&base_freq, &turbo_freq))
goto out;
--
2.31.1
On Fri, May 20, 2022 at 06:10:21PM +0200, Giovanni Gherdovich wrote:
> knl_set_max_freq_ratio() should return true on success and false
> otherwise. If the last line of the function body is reached, it means no
> appropriate value for turbo_freq was found: the setup is unsuccessful and
> it should return false.
>
> Fixes: 8bea0dfb4a82 ("x86, sched: Add support for frequency invariance on XEON_PHI_KNL/KNM")
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Giovanni Gherdovich <[email protected]>
> ---
> arch/x86/kernel/smpboot.c | 2 +-
You seems to have missed all that code moved in tip. This no longer
applies.
On Sat, 2022-05-21 at 12:02 +0200, Peter Zijlstra wrote:
> On Fri, May 20, 2022 at 06:10:21PM +0200, Giovanni Gherdovich wrote:
> > knl_set_max_freq_ratio() should return true on success and false
> > otherwise. If the last line of the function body is reached, it means no
> > appropriate value for turbo_freq was found: the setup is unsuccessful and
> > it should return false.
> >
> > Fixes: 8bea0dfb4a82 ("x86, sched: Add support for frequency invariance on XEON_PHI_KNL/KNM")
> > Reported-by: Dan Carpenter <[email protected]>
> > Signed-off-by: Giovanni Gherdovich <[email protected]>
> > ---
> > arch/x86/kernel/smpboot.c | 2 +-
>
> You seems to have missed all that code moved in tip. This no longer
> applies.
Right. I'll rebase and resend. Plus, as per Dave Hansen's comments on
the other patch, I need to have a second look at it.
Giovanni