[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <PH0PR11MB48245F82B9F5352878FD84B5CD049@PH0PR11MB4824.namprd11.prod.outlook.com>
Date: Tue, 15 Nov 2022 06:25:24 +0000
From: "Mi, Dapeng1" <dapeng1.mi@...el.com>
To: "Christopherson,, Sean" <seanjc@...gle.com>
CC: "pbonzini@...hat.com" <pbonzini@...hat.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"zhenyuw@...ux.intel.com" <zhenyuw@...ux.intel.com>
Subject: RE: [PATCH] KVM: x86: disable halt polling when powersave governor is
used
> From: Sean Christopherson <seanjc@...gle.com>
> Sent: Wednesday, October 26, 2022 1:07 AM
> To: Mi, Dapeng1 <dapeng1.mi@...el.com>
> Cc: pbonzini@...hat.com; tglx@...utronix.de; mingo@...hat.com;
> dave.hansen@...ux.intel.com; kvm@...r.kernel.org; linux-
> kernel@...r.kernel.org; zhenyuw@...ux.intel.com
> Subject: Re: [PATCH] KVM: x86: disable halt polling when powersave
> governor is used
>
> On Sat, Oct 08, 2022, Mi, Dapeng1 wrote:
> > > > + !strncmp(policy->governor->name,
> > > "powersave",
> > >
> > > KVM should not be comparing magic strings. If the cpufreq subsystem
> > > can't get
> > > policy->policy right, then that needs to be fixed.
> >
> > Yeah, using magic strings looks a little bit strange, but this is what
> > is cpufreq doing. Currently cpufreq mechanism supports two kinds of
> > drivers, one is the driver which has the built-in governor, like intel_pstate
> driver.
> > For this kind of driver, the cpufreq governor is saved in the
> > policy->policy field. The other is the traditional driver which is
> > independent with cpufreq governor and the cpufreq governor type is
> saved in the governor->name field.
> > For the second kind of cpufreq driver, the policy->policy field is
> > meaningless and we have to read the governor name.
>
> That doesn't mean it's ok to bleed those internal details into KVM. I would
> much rather cpufreq provide a helper to get the effective policy, e.g.
>
> unsigned int cpufreq_cpu_get_policy(unsigned int cpu)
> {
> struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> unsigned int pol;
>
> if (!policy)
> return CPUFREQ_POLICY_UNKNOWN;
>
> pol = policy->policy
> if (pol == CPUFREQ_POLICY_UNKNOWN && policy->governor)
> pol = cpufreq_parse_policy(policy->governor->name);
>
> cpufreq_cpu_put(policy);
> }
Thanks Sean for reviewing. Would do in next version.
Powered by blists - more mailing lists