Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH] cpuidle-haltpoll: Enable kvm guest polling when dedicated physical CPUs are available
       [not found] <1564643196-7797-1-git-send-email-wanpengli@tencent.com>
@ 2019-08-01 16:51 ` Rafael J. Wysocki
  2019-08-01 16:54   ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Rafael J. Wysocki @ 2019-08-01 16:51 UTC (permalink / raw)
  To: Wanpeng Li, Paolo Bonzini, Marcelo Tosatti
  Cc: linux-kernel, kvm, Radim Krčmář, Linux PM

On 8/1/2019 9:06 AM, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
>
> The downside of guest side polling is that polling is performed even
> with other runnable tasks in the host. However, even if poll in kvm
> can aware whether or not other runnable tasks in the same pCPU, it
> can still incur extra overhead in over-subscribe scenario. Now we can
> just enable guest polling when dedicated pCPUs are available.
>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>

Paolo, Marcelo, any comments?


> ---
>   drivers/cpuidle/cpuidle-haltpoll.c   | 3 ++-
>   drivers/cpuidle/governors/haltpoll.c | 2 +-
>   2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
> index 9ac093d..7aee38a 100644
> --- a/drivers/cpuidle/cpuidle-haltpoll.c
> +++ b/drivers/cpuidle/cpuidle-haltpoll.c
> @@ -53,7 +53,8 @@ static int __init haltpoll_init(void)
>   
>   	cpuidle_poll_state_init(drv);
>   
> -	if (!kvm_para_available())
> +	if (!kvm_para_available() ||
> +		!kvm_para_has_hint(KVM_HINTS_REALTIME))
>   		return 0;
>   
>   	ret = cpuidle_register(&haltpoll_driver, NULL);
> diff --git a/drivers/cpuidle/governors/haltpoll.c b/drivers/cpuidle/governors/haltpoll.c
> index 797477b..685c7007 100644
> --- a/drivers/cpuidle/governors/haltpoll.c
> +++ b/drivers/cpuidle/governors/haltpoll.c
> @@ -141,7 +141,7 @@ static struct cpuidle_governor haltpoll_governor = {
>   
>   static int __init init_haltpoll(void)
>   {
> -	if (kvm_para_available())
> +	if (kvm_para_available() && kvm_para_has_hint(KVM_HINTS_REALTIME))
>   		return cpuidle_register_governor(&haltpoll_governor);
>   
>   	return 0;



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpuidle-haltpoll: Enable kvm guest polling when dedicated physical CPUs are available
  2019-08-01 16:51 ` [PATCH] cpuidle-haltpoll: Enable kvm guest polling when dedicated physical CPUs are available Rafael J. Wysocki
@ 2019-08-01 16:54   ` Paolo Bonzini
  2019-08-03 20:21     ` Marcelo Tosatti
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2019-08-01 16:54 UTC (permalink / raw)
  To: Rafael J. Wysocki, Wanpeng Li, Marcelo Tosatti
  Cc: linux-kernel, kvm, Radim Krčmář, Linux PM

On 01/08/19 18:51, Rafael J. Wysocki wrote:
> On 8/1/2019 9:06 AM, Wanpeng Li wrote:
>> From: Wanpeng Li <wanpengli@tencent.com>
>>
>> The downside of guest side polling is that polling is performed even
>> with other runnable tasks in the host. However, even if poll in kvm
>> can aware whether or not other runnable tasks in the same pCPU, it
>> can still incur extra overhead in over-subscribe scenario. Now we can
>> just enable guest polling when dedicated pCPUs are available.
>>
>> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Marcelo Tosatti <mtosatti@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> 
> Paolo, Marcelo, any comments?

Yes, it's a good idea.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

> 
>> ---
>>   drivers/cpuidle/cpuidle-haltpoll.c   | 3 ++-
>>   drivers/cpuidle/governors/haltpoll.c | 2 +-
>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle-haltpoll.c
>> b/drivers/cpuidle/cpuidle-haltpoll.c
>> index 9ac093d..7aee38a 100644
>> --- a/drivers/cpuidle/cpuidle-haltpoll.c
>> +++ b/drivers/cpuidle/cpuidle-haltpoll.c
>> @@ -53,7 +53,8 @@ static int __init haltpoll_init(void)
>>         cpuidle_poll_state_init(drv);
>>   -    if (!kvm_para_available())
>> +    if (!kvm_para_available() ||
>> +        !kvm_para_has_hint(KVM_HINTS_REALTIME))
>>           return 0;
>>         ret = cpuidle_register(&haltpoll_driver, NULL);
>> diff --git a/drivers/cpuidle/governors/haltpoll.c
>> b/drivers/cpuidle/governors/haltpoll.c
>> index 797477b..685c7007 100644
>> --- a/drivers/cpuidle/governors/haltpoll.c
>> +++ b/drivers/cpuidle/governors/haltpoll.c
>> @@ -141,7 +141,7 @@ static struct cpuidle_governor haltpoll_governor = {
>>     static int __init init_haltpoll(void)
>>   {
>> -    if (kvm_para_available())
>> +    if (kvm_para_available() && kvm_para_has_hint(KVM_HINTS_REALTIME))
>>           return cpuidle_register_governor(&haltpoll_governor);
>>         return 0;
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpuidle-haltpoll: Enable kvm guest polling when dedicated physical CPUs are available
  2019-08-01 16:54   ` Paolo Bonzini
@ 2019-08-03 20:21     ` Marcelo Tosatti
  2019-08-13  0:55       ` Wanpeng Li
  0 siblings, 1 reply; 5+ messages in thread
From: Marcelo Tosatti @ 2019-08-03 20:21 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Rafael J. Wysocki, Wanpeng Li, linux-kernel, kvm,
	Radim Krčmář,
	Linux PM

On Thu, Aug 01, 2019 at 06:54:49PM +0200, Paolo Bonzini wrote:
> On 01/08/19 18:51, Rafael J. Wysocki wrote:
> > On 8/1/2019 9:06 AM, Wanpeng Li wrote:
> >> From: Wanpeng Li <wanpengli@tencent.com>
> >>
> >> The downside of guest side polling is that polling is performed even
> >> with other runnable tasks in the host. However, even if poll in kvm
> >> can aware whether or not other runnable tasks in the same pCPU, it
> >> can still incur extra overhead in over-subscribe scenario. Now we can
> >> just enable guest polling when dedicated pCPUs are available.
> >>
> >> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >> Cc: Radim Krčmář <rkrcmar@redhat.com>
> >> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> >> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> > 
> > Paolo, Marcelo, any comments?
> 
> Yes, it's a good idea.
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Paolo

I think KVM_HINTS_REALTIME is being abused somewhat.
It has no clear meaning and used in different locations 
for different purposes.

For example, i think that using pv queued spinlocks and 
haltpoll is a desired scenario, which the patch below disallows.

Wanpeng Li, currently the driver does not autoload. So polling in 
the guest has to be enabled manually. Isnt that sufficient?



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpuidle-haltpoll: Enable kvm guest polling when dedicated physical CPUs are available
  2019-08-03 20:21     ` Marcelo Tosatti
@ 2019-08-13  0:55       ` Wanpeng Li
  2019-08-13  7:55         ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Wanpeng Li @ 2019-08-13  0:55 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Paolo Bonzini, Rafael J. Wysocki, LKML, kvm,
	Radim Krčmář,
	Linux PM

On Sun, 4 Aug 2019 at 04:21, Marcelo Tosatti <mtosatti@redhat.com> wrote:
>
> On Thu, Aug 01, 2019 at 06:54:49PM +0200, Paolo Bonzini wrote:
> > On 01/08/19 18:51, Rafael J. Wysocki wrote:
> > > On 8/1/2019 9:06 AM, Wanpeng Li wrote:
> > >> From: Wanpeng Li <wanpengli@tencent.com>
> > >>
> > >> The downside of guest side polling is that polling is performed even
> > >> with other runnable tasks in the host. However, even if poll in kvm
> > >> can aware whether or not other runnable tasks in the same pCPU, it
> > >> can still incur extra overhead in over-subscribe scenario. Now we can
> > >> just enable guest polling when dedicated pCPUs are available.
> > >>
> > >> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >> Cc: Paolo Bonzini <pbonzini@redhat.com>
> > >> Cc: Radim Krčmář <rkrcmar@redhat.com>
> > >> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> > >> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> > >
> > > Paolo, Marcelo, any comments?
> >
> > Yes, it's a good idea.
> >
> > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> >
> > Paolo
>

Hi Marcelo,

Sorry for the late response.

> I think KVM_HINTS_REALTIME is being abused somewhat.
> It has no clear meaning and used in different locations
> for different purposes.

================== ============ =================================
KVM_HINTS_REALTIME 0                      guest checks this feature bit to

determine that vCPUs are never

preempted for an unlimited time

allowing optimizations
================== ============ =================================

Now it disables pv queued spinlock, pv tlb shootdown, pv sched yield
which are not expected present in vCPUs are never preempted for an
unlimited time scenario.

>
> For example, i think that using pv queued spinlocks and
> haltpoll is a desired scenario, which the patch below disallows.

So even if dedicated pCPU is available, pv queued spinlocks should
still be chose if something like vhost-kthreads are used instead of
DPDK/vhost-user. kvm adaptive halt-polling will compete with
vhost-kthreads, however, poll in guest unaware other runnable tasks in
the host which will defeat vhost-kthreads.

Regards,
Wanpeng Li

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpuidle-haltpoll: Enable kvm guest polling when dedicated physical CPUs are available
  2019-08-13  0:55       ` Wanpeng Li
@ 2019-08-13  7:55         ` Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2019-08-13  7:55 UTC (permalink / raw)
  To: Wanpeng Li, Marcelo Tosatti
  Cc: Rafael J. Wysocki, LKML, kvm, Radim Krčmář, Linux PM

On 13/08/19 02:55, Wanpeng Li wrote:
>> I think KVM_HINTS_REALTIME is being abused somewhat.
>> It has no clear meaning and used in different locations
>> for different purposes.
> 
> Now it disables pv queued spinlock, pv tlb shootdown, pv sched yield
> which are not expected present in vCPUs are never preempted for an
> unlimited time scenario.

Guest side polling definitely matches the purpose of KVM_HINTS_REALTIME.
 While host-side polling is conditional on single_task_running, this is
obviously not true of guest-side polling.

The alternative would be to enable it only if KVM_FEATURE_POLL_CONTROL
is available, but I prefer Wanpeng's patch.

Paolo

>> For example, i think that using pv queued spinlocks and
>> haltpoll is a desired scenario, which the patch below disallows.
> 
> So even if dedicated pCPU is available, pv queued spinlocks should
> still be chose if something like vhost-kthreads are used instead of
> DPDK/vhost-user. kvm adaptive halt-polling will compete with
> vhost-kthreads, however, poll in guest unaware other runnable tasks in
> the host which will defeat vhost-kthreads.
> 
> Regards,
> Wanpeng Li
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1564643196-7797-1-git-send-email-wanpengli@tencent.com>
2019-08-01 16:51 ` [PATCH] cpuidle-haltpoll: Enable kvm guest polling when dedicated physical CPUs are available Rafael J. Wysocki
2019-08-01 16:54   ` Paolo Bonzini
2019-08-03 20:21     ` Marcelo Tosatti
2019-08-13  0:55       ` Wanpeng Li
2019-08-13  7:55         ` Paolo Bonzini

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org linux-pm@archiver.kernel.org
	public-inbox-index linux-pm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox