From nobody Fri Apr 17 15:34:07 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8D049C4332F for ; Sun, 4 Dec 2022 12:05:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230056AbiLDMFX convert rfc822-to-8bit (ORCPT ); Sun, 4 Dec 2022 07:05:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55164 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229753AbiLDMFT (ORCPT ); Sun, 4 Dec 2022 07:05:19 -0500 X-Greylist: delayed 915 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Sun, 04 Dec 2022 04:05:17 PST Received: from baidu.com (mx20.baidu.com [111.202.115.85]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 34519175A8 for ; Sun, 4 Dec 2022 04:05:16 -0800 (PST) From: "Li,Rongqing" To: Thomas Gleixner , "mingo@redhat.com" , "bp@alien8.de" , "dave.hansen@linux.intel.com" , "x86@kernel.org" , "peterz@infradead.org" , "tony.luck@intel.com" , "wyes.karny@amd.com" , "linux-kernel@vger.kernel.org" , "rafael.j.wysocki@intel.com" Subject: RE: [PATCH][RFC] x86: override prefer_mwait_c1_over_halt to avoid loading cpuidle-haltpoll driver Thread-Topic: [PATCH][RFC] x86: override prefer_mwait_c1_over_halt to avoid loading cpuidle-haltpoll driver Thread-Index: AQHZBn6jYStMiLz0YUyj1x+4POZvoq5dmZ8g Date: Sun, 4 Dec 2022 11:31:27 +0000 Message-ID: References: <1669952252-32710-1-git-send-email-lirongqing@baidu.com> <87fsdxprpr.ffs@tglx> In-Reply-To: <87fsdxprpr.ffs@tglx> Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.14.117.44] x-baidu-bdmsfe-datecheck: 1_BJHW-Mail-Ex15_2022-12-04 19:31:27:660 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-FEAS-Client-IP: 10.127.64.38 X-FE-Last-Public-Client-IP: 100.100.100.38 X-FE-Policy-ID: 15:10:21:SYSTEM Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" > -----Original Message----- > From: Thomas Gleixner > Sent: Saturday, December 3, 2022 2:48 AM > To: Li,Rongqing ; mingo@redhat.com; bp@alien8.de; > dave.hansen@linux.intel.com; x86@kernel.org; peterz@infradead.org; > tony.luck@intel.com; wyes.karny@amd.com; linux-kernel@vger.kernel.org; > rafael.j.wysocki@intel.com > Subject: Re: [PATCH][RFC] x86: override prefer_mwait_c1_over_halt to avoid > loading cpuidle-haltpoll driver >=20 > Li! >=20 > On Fri, Dec 02 2022 at 11:37, lirongqing@baidu.com wrote: > > From: Li RongQing > > > > x86 KVM guests with MWAIT can load cpuidle-haltpoll driver, and will > > cause performance degradation, so override prefer_mwait_c1_over_halt > > to a new value, aviod loading cpuidle-haltpoll driver >=20 > Neither the subject line nor the above makes any sense to me. >=20 > prefer_mwait_c1_over_halt() is a function which is invoked and when it re= turns > true then the execution ends up in the code path you are patching. >=20 > > @@ -889,6 +889,7 @@ void select_idle_routine(const struct cpuinfo_x86 *= c) > > } else if (prefer_mwait_c1_over_halt(c)) { > > pr_info("using mwait in idle threads\n"); > > x86_idle =3D mwait_idle; > > + boot_option_idle_override =3D IDLE_PREF_MWAIT; >=20 > What you do is setting boot_option_idle_override to a new value, but that= has > nothing to do with prefer_mwait_c1_over_halt() at all. >=20 > So how are you overriding that function to a new value? >=20 > But that's just a word smithing problem. >=20 > The real and way worse problem is that you pick a variable, which has the > purpose to capture the idle override on the kernel command line, and modi= fy it > as you see fit, just to prevent that driver from loading. >=20 > select_idle_routine() reads it to check whether there was a command line > override or not. But it is not supposed to write it. Why? >=20 > Have you checked what else evaluates that variable? Obviously not, becaus= e a > simple grep would have told you: >=20 > drivers/cpuidle/cpuidle-haltpoll.c: if (boot_option_idle_override != =3D > IDLE_NO_OVERRIDE) > drivers/idle/intel_idle.c: if (boot_option_idle_override !=3D > IDLE_NO_OVERRIDE) >=20 > Congratulations! >=20 > Your patch breaks the default setup of every recent Intel system on the p= lanet > because it not only prevents the cpuidle-haltpoll, but also the intel_idl= e driver > from loading. >=20 > Seriously. It's not too much asked to do at least a simple grep and look = at all > _nine_ places which evaluate boot_option_idle_override. >=20 Sorry for the careless Thanks for the review, I will send a new version, which export a function t= o tell haltpoll driver whether or not mwait is used, like below diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/proces= sor.h index 67c9d73..159ef33 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -862,4 +862,6 @@ bool arch_is_platform_page(u64 paddr); #define arch_is_platform_page arch_is_platform_page #endif +bool is_mwait_idle(void); + #endif /* _ASM_X86_PROCESSOR_H */ diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c index c21b734..330972c 100644 --- a/arch/x86/kernel/process.c +++ b/arch/x86/kernel/process.c @@ -896,6 +896,12 @@ void select_idle_routine(const struct cpuinfo_x86 *c) x86_idle =3D default_idle; } +bool is_mwait_idle(void) +{ + return x86_idle =3D=3D mwait_idle; +} +EXPORT_SYMBOL_GPL(is_mwait_idle); + void amd_e400_c1e_apic_setup(void) { if (boot_cpu_has_bug(X86_BUG_AMD_APIC_C1E)) { diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-h= altpoll.c index 3a39a7f..8cf1ddf 100644 --- a/drivers/cpuidle/cpuidle-haltpoll.c +++ b/drivers/cpuidle/cpuidle-haltpoll.c @@ -17,6 +17,7 @@ #include #include #include +#include static bool force __read_mostly; module_param(force, bool, 0444); @@ -111,6 +112,9 @@ static int __init haltpoll_init(void) if (!kvm_para_available() || !haltpoll_want()) return -ENODEV; + if (is_mwait_idle()) + return -ENODEV; + cpuidle_poll_state_init(drv); ret =3D cpuidle_register_driver(drv); Thanks -Li