Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Juergen Gross]

On 13/11/17 11:06, Quan Xu wrote:
> From: Quan Xu <quan.xu0@gmail.com>
> 
> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
> in idle path which will poll for a while before we enter the real idle
> state.
> 
> In virtualization, idle path includes several heavy operations
> includes timer access(LAPIC timer or TSC deadline timer) which will
> hurt performance especially for latency intensive workload like message
> passing task. The cost is mainly from the vmexit which is a hardware
> context switch between virtual machine and hypervisor. Our solution is
> to poll for a while and do not enter real idle path if we can get the
> schedule event during polling.
> 
> Poll may cause the CPU waste so we adopt a smart polling mechanism to
> reduce the useless poll.
> 
> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Alok Kataria <akataria@vmware.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: virtualization@lists.linux-foundation.org
> Cc: linux-kernel@vger.kernel.org
> Cc: xen-devel@lists.xenproject.org

Hmm, is the idle entry path really so critical to performance that a new
pvops function is necessary? Wouldn't a function pointer, maybe guarded
by a static key, be enough? A further advantage would be that this would
work on other architectures, too.


Juergen

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Wanpeng Li]

2017-11-13 18:53 GMT+08:00 Juergen Gross <jgross@suse.com>:
> On 13/11/17 11:06, Quan Xu wrote:
>> From: Quan Xu <quan.xu0@gmail.com>
>>
>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
>> in idle path which will poll for a while before we enter the real idle
>> state.
>>
>> In virtualization, idle path includes several heavy operations
>> includes timer access(LAPIC timer or TSC deadline timer) which will
>> hurt performance especially for latency intensive workload like message
>> passing task. The cost is mainly from the vmexit which is a hardware
>> context switch between virtual machine and hypervisor. Our solution is
>> to poll for a while and do not enter real idle path if we can get the
>> schedule event during polling.
>>
>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>> reduce the useless poll.
>>
>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>> Cc: Juergen Gross <jgross@suse.com>
>> Cc: Alok Kataria <akataria@vmware.com>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: x86@kernel.org
>> Cc: virtualization@lists.linux-foundation.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: xen-devel@lists.xenproject.org
>
> Hmm, is the idle entry path really so critical to performance that a new
> pvops function is necessary? Wouldn't a function pointer, maybe guarded
> by a static key, be enough? A further advantage would be that this would
> work on other architectures, too.

There is a "Adaptive halt-polling" which are merged to upstream more
than two years ago avoids to thread the critical path and has already
been ported to other architectures. https://lkml.org/lkml/2015/9/3/615

Regards,
Wanpeng Li

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Quan Xu]

On 2017/11/13 18:53, Juergen Gross wrote:
> On 13/11/17 11:06, Quan Xu wrote:
>> From: Quan Xu <quan.xu0@gmail.com>
>>
>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
>> in idle path which will poll for a while before we enter the real idle
>> state.
>>
>> In virtualization, idle path includes several heavy operations
>> includes timer access(LAPIC timer or TSC deadline timer) which will
>> hurt performance especially for latency intensive workload like message
>> passing task. The cost is mainly from the vmexit which is a hardware
>> context switch between virtual machine and hypervisor. Our solution is
>> to poll for a while and do not enter real idle path if we can get the
>> schedule event during polling.
>>
>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>> reduce the useless poll.
>>
>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>> Cc: Juergen Gross <jgross@suse.com>
>> Cc: Alok Kataria <akataria@vmware.com>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: x86@kernel.org
>> Cc: virtualization@lists.linux-foundation.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: xen-devel@lists.xenproject.org
> Hmm, is the idle entry path really so critical to performance that a new
> pvops function is necessary?
Juergen, Here is the data we get when running benchmark netperf:
  1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
     29031.6 bit/s -- 76.1 %CPU

  2. w/ patch and disable kvm dynamic poll (halt_poll_ns=0):
     35787.7 bit/s -- 129.4 %CPU

  3. w/ kvm dynamic poll:
     35735.6 bit/s -- 200.0 %CPU

  4. w/patch and w/ kvm dynamic poll:
     42225.3 bit/s -- 198.7 %CPU

  5. idle=poll
     37081.7 bit/s -- 998.1 %CPU



  w/ this patch, we will improve performance by 23%.. even we could improve
  performance by 45.4%, if we use w/patch and w/ kvm dynamic poll. also the
  cost of CPU is much lower than 'idle=poll' case..

> Wouldn't a function pointer, maybe guarded
> by a static key, be enough? A further advantage would be that this would
> work on other architectures, too.

I assume this feature will be ported to other archs.. a new pvops makes code
clean and easy to maintain. also I tried to add it into existed pvops, 
but it
doesn't match.



Quan
Alibaba Cloud
>
> Juergen
>

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Wanpeng Li]

2017-11-14 15:02 GMT+08:00 Quan Xu <quan.xu0@gmail.com>:
>
>
> On 2017/11/13 18:53, Juergen Gross wrote:
>>
>> On 13/11/17 11:06, Quan Xu wrote:
>>>
>>> From: Quan Xu <quan.xu0@gmail.com>
>>>
>>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
>>> in idle path which will poll for a while before we enter the real idle
>>> state.
>>>
>>> In virtualization, idle path includes several heavy operations
>>> includes timer access(LAPIC timer or TSC deadline timer) which will
>>> hurt performance especially for latency intensive workload like message
>>> passing task. The cost is mainly from the vmexit which is a hardware
>>> context switch between virtual machine and hypervisor. Our solution is
>>> to poll for a while and do not enter real idle path if we can get the
>>> schedule event during polling.
>>>
>>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>>> reduce the useless poll.
>>>
>>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>>> Cc: Juergen Gross <jgross@suse.com>
>>> Cc: Alok Kataria <akataria@vmware.com>
>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>> Cc: x86@kernel.org
>>> Cc: virtualization@lists.linux-foundation.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: xen-devel@lists.xenproject.org
>>
>> Hmm, is the idle entry path really so critical to performance that a new
>> pvops function is necessary?
>
> Juergen, Here is the data we get when running benchmark netperf:
>  1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>     29031.6 bit/s -- 76.1 %CPU
>
>  2. w/ patch and disable kvm dynamic poll (halt_poll_ns=0):
>     35787.7 bit/s -- 129.4 %CPU
>
>  3. w/ kvm dynamic poll:
>     35735.6 bit/s -- 200.0 %CPU

Actually we can reduce the CPU utilization by sleeping a period of
time as what has already been done in the poll logic of IO subsystem,
then we can improve the algorithm in kvm instead of introduing another
duplicate one in the kvm guest.

Regards,
Wanpeng Li

>
>  4. w/patch and w/ kvm dynamic poll:
>     42225.3 bit/s -- 198.7 %CPU
>
>  5. idle=poll
>     37081.7 bit/s -- 998.1 %CPU
>
>
>
>  w/ this patch, we will improve performance by 23%.. even we could improve
>  performance by 45.4%, if we use w/patch and w/ kvm dynamic poll. also the
>  cost of CPU is much lower than 'idle=poll' case..
>
>> Wouldn't a function pointer, maybe guarded
>> by a static key, be enough? A further advantage would be that this would
>> work on other architectures, too.
>
>
> I assume this feature will be ported to other archs.. a new pvops makes code
> clean and easy to maintain. also I tried to add it into existed pvops, but
> it
> doesn't match.
>
>
>
> Quan
> Alibaba Cloud
>>
>>
>> Juergen
>>
>

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Quan Xu]

On 2017/11/14 15:12, Wanpeng Li wrote:
> 2017-11-14 15:02 GMT+08:00 Quan Xu <quan.xu0@gmail.com>:
>>
>> On 2017/11/13 18:53, Juergen Gross wrote:
>>> On 13/11/17 11:06, Quan Xu wrote:
>>>> From: Quan Xu <quan.xu0@gmail.com>
>>>>
>>>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
>>>> in idle path which will poll for a while before we enter the real idle
>>>> state.
>>>>
>>>> In virtualization, idle path includes several heavy operations
>>>> includes timer access(LAPIC timer or TSC deadline timer) which will
>>>> hurt performance especially for latency intensive workload like message
>>>> passing task. The cost is mainly from the vmexit which is a hardware
>>>> context switch between virtual machine and hypervisor. Our solution is
>>>> to poll for a while and do not enter real idle path if we can get the
>>>> schedule event during polling.
>>>>
>>>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>>>> reduce the useless poll.
>>>>
>>>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>>>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>>>> Cc: Juergen Gross <jgross@suse.com>
>>>> Cc: Alok Kataria <akataria@vmware.com>
>>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>> Cc: Ingo Molnar <mingo@redhat.com>
>>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>>> Cc: x86@kernel.org
>>>> Cc: virtualization@lists.linux-foundation.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Cc: xen-devel@lists.xenproject.org
>>> Hmm, is the idle entry path really so critical to performance that a new
>>> pvops function is necessary?
>> Juergen, Here is the data we get when running benchmark netperf:
>>   1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>>      29031.6 bit/s -- 76.1 %CPU
>>
>>   2. w/ patch and disable kvm dynamic poll (halt_poll_ns=0):
>>      35787.7 bit/s -- 129.4 %CPU
>>
>>   3. w/ kvm dynamic poll:
>>      35735.6 bit/s -- 200.0 %CPU
> Actually we can reduce the CPU utilization by sleeping a period of
> time as what has already been done in the poll logic of IO subsystem,
> then we can improve the algorithm in kvm instead of introduing another
> duplicate one in the kvm guest.
We really appreciate upstream's kvm dynamic poll mechanism, which is
really helpful for a lot of scenario..

However, as description said, in virtualization, idle path includes
several heavy operations includes timer access (LAPIC timer or TSC
deadline timer) which will hurt performance especially for latency
intensive workload like message passing task. The cost is mainly from
the vmexit which is a hardware context switch between virtual machine
and hypervisor.

for upstream's kvm dynamic poll mechanism, even you could provide a
better algorism, how could you bypass timer access (LAPIC timer or TSC
deadline timer), or a hardware context switch between virtual machine
and hypervisor. I know these is a tradeoff.

Furthermore, here is the data we get when running benchmark contextswitch
to measure the latency(lower is better):

1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
   3402.9 ns/ctxsw -- 199.8 %CPU

2. w/ patch and disable kvm dynamic poll:
   1163.5 ns/ctxsw -- 205.5 %CPU

3. w/ kvm dynamic poll:
   2280.6 ns/ctxsw -- 199.5 %CPU

so, these tow solution are quite similar, but not duplicate..

that's also why to add a generic idle poll before enter real idle path.
When a reschedule event is pending, we can bypass the real idle path.


Quan
Alibaba Cloud




> Regards,
> Wanpeng Li
>
>>   4. w/patch and w/ kvm dynamic poll:
>>      42225.3 bit/s -- 198.7 %CPU
>>
>>   5. idle=poll
>>      37081.7 bit/s -- 998.1 %CPU
>>
>>
>>
>>   w/ this patch, we will improve performance by 23%.. even we could improve
>>   performance by 45.4%, if we use w/patch and w/ kvm dynamic poll. also the
>>   cost of CPU is much lower than 'idle=poll' case..
>>
>>> Wouldn't a function pointer, maybe guarded
>>> by a static key, be enough? A further advantage would be that this would
>>> work on other architectures, too.
>>
>> I assume this feature will be ported to other archs.. a new pvops makes code
>> clean and easy to maintain. also I tried to add it into existed pvops, but
>> it
>> doesn't match.
>>
>>
>>
>> Quan
>> Alibaba Cloud
>>>
>>> Juergen
>>>

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Wanpeng Li]

2017-11-14 16:15 GMT+08:00 Quan Xu <quan.xu0@gmail.com>:
>
>
> On 2017/11/14 15:12, Wanpeng Li wrote:
>>
>> 2017-11-14 15:02 GMT+08:00 Quan Xu <quan.xu0@gmail.com>:
>>>
>>>
>>> On 2017/11/13 18:53, Juergen Gross wrote:
>>>>
>>>> On 13/11/17 11:06, Quan Xu wrote:
>>>>>
>>>>> From: Quan Xu <quan.xu0@gmail.com>
>>>>>
>>>>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
>>>>> in idle path which will poll for a while before we enter the real idle
>>>>> state.
>>>>>
>>>>> In virtualization, idle path includes several heavy operations
>>>>> includes timer access(LAPIC timer or TSC deadline timer) which will
>>>>> hurt performance especially for latency intensive workload like message
>>>>> passing task. The cost is mainly from the vmexit which is a hardware
>>>>> context switch between virtual machine and hypervisor. Our solution is
>>>>> to poll for a while and do not enter real idle path if we can get the
>>>>> schedule event during polling.
>>>>>
>>>>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>>>>> reduce the useless poll.
>>>>>
>>>>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>>>>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>>>>> Cc: Juergen Gross <jgross@suse.com>
>>>>> Cc: Alok Kataria <akataria@vmware.com>
>>>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>>> Cc: Ingo Molnar <mingo@redhat.com>
>>>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>>>> Cc: x86@kernel.org
>>>>> Cc: virtualization@lists.linux-foundation.org
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Cc: xen-devel@lists.xenproject.org
>>>>
>>>> Hmm, is the idle entry path really so critical to performance that a new
>>>> pvops function is necessary?
>>>
>>> Juergen, Here is the data we get when running benchmark netperf:
>>>   1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>>>      29031.6 bit/s -- 76.1 %CPU
>>>
>>>   2. w/ patch and disable kvm dynamic poll (halt_poll_ns=0):
>>>      35787.7 bit/s -- 129.4 %CPU
>>>
>>>   3. w/ kvm dynamic poll:
>>>      35735.6 bit/s -- 200.0 %CPU
>>
>> Actually we can reduce the CPU utilization by sleeping a period of
>> time as what has already been done in the poll logic of IO subsystem,
>> then we can improve the algorithm in kvm instead of introduing another
>> duplicate one in the kvm guest.
>
> We really appreciate upstream's kvm dynamic poll mechanism, which is
> really helpful for a lot of scenario..
>
> However, as description said, in virtualization, idle path includes
> several heavy operations includes timer access (LAPIC timer or TSC
> deadline timer) which will hurt performance especially for latency
> intensive workload like message passing task. The cost is mainly from
> the vmexit which is a hardware context switch between virtual machine
> and hypervisor.
>
> for upstream's kvm dynamic poll mechanism, even you could provide a
> better algorism, how could you bypass timer access (LAPIC timer or TSC
> deadline timer), or a hardware context switch between virtual machine
> and hypervisor. I know these is a tradeoff.
>
> Furthermore, here is the data we get when running benchmark contextswitch
> to measure the latency(lower is better):
>
> 1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>   3402.9 ns/ctxsw -- 199.8 %CPU
>
> 2. w/ patch and disable kvm dynamic poll:
>   1163.5 ns/ctxsw -- 205.5 %CPU
>
> 3. w/ kvm dynamic poll:
>   2280.6 ns/ctxsw -- 199.5 %CPU
>
> so, these tow solution are quite similar, but not duplicate..
>
> that's also why to add a generic idle poll before enter real idle path.
> When a reschedule event is pending, we can bypass the real idle path.
>

There is a similar logic in the idle governor/driver, so how this
patchset influence the decision in the idle governor/driver when
running on bare-metal(power managment is not exposed to the guest so
we will not enter into idle driver in the guest)?

Regards,
Wanpeng Li

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Quan Xu]

On 2017/11/14 16:22, Wanpeng Li wrote:
> 2017-11-14 16:15 GMT+08:00 Quan Xu <quan.xu0@gmail.com>:
>>
>> On 2017/11/14 15:12, Wanpeng Li wrote:
>>> 2017-11-14 15:02 GMT+08:00 Quan Xu <quan.xu0@gmail.com>:
>>>>
>>>> On 2017/11/13 18:53, Juergen Gross wrote:
>>>>> On 13/11/17 11:06, Quan Xu wrote:
>>>>>> From: Quan Xu <quan.xu0@gmail.com>
>>>>>>
>>>>>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
>>>>>> in idle path which will poll for a while before we enter the real idle
>>>>>> state.
>>>>>>
>>>>>> In virtualization, idle path includes several heavy operations
>>>>>> includes timer access(LAPIC timer or TSC deadline timer) which will
>>>>>> hurt performance especially for latency intensive workload like message
>>>>>> passing task. The cost is mainly from the vmexit which is a hardware
>>>>>> context switch between virtual machine and hypervisor. Our solution is
>>>>>> to poll for a while and do not enter real idle path if we can get the
>>>>>> schedule event during polling.
>>>>>>
>>>>>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>>>>>> reduce the useless poll.
>>>>>>
>>>>>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>>>>>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>>>>>> Cc: Juergen Gross <jgross@suse.com>
>>>>>> Cc: Alok Kataria <akataria@vmware.com>
>>>>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>>>> Cc: Ingo Molnar <mingo@redhat.com>
>>>>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>>>>> Cc: x86@kernel.org
>>>>>> Cc: virtualization@lists.linux-foundation.org
>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>> Cc: xen-devel@lists.xenproject.org
>>>>> Hmm, is the idle entry path really so critical to performance that a new
>>>>> pvops function is necessary?
>>>> Juergen, Here is the data we get when running benchmark netperf:
>>>>    1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>>>>       29031.6 bit/s -- 76.1 %CPU
>>>>
>>>>    2. w/ patch and disable kvm dynamic poll (halt_poll_ns=0):
>>>>       35787.7 bit/s -- 129.4 %CPU
>>>>
>>>>    3. w/ kvm dynamic poll:
>>>>       35735.6 bit/s -- 200.0 %CPU
>>> Actually we can reduce the CPU utilization by sleeping a period of
>>> time as what has already been done in the poll logic of IO subsystem,
>>> then we can improve the algorithm in kvm instead of introduing another
>>> duplicate one in the kvm guest.
>> We really appreciate upstream's kvm dynamic poll mechanism, which is
>> really helpful for a lot of scenario..
>>
>> However, as description said, in virtualization, idle path includes
>> several heavy operations includes timer access (LAPIC timer or TSC
>> deadline timer) which will hurt performance especially for latency
>> intensive workload like message passing task. The cost is mainly from
>> the vmexit which is a hardware context switch between virtual machine
>> and hypervisor.
>>
>> for upstream's kvm dynamic poll mechanism, even you could provide a
>> better algorism, how could you bypass timer access (LAPIC timer or TSC
>> deadline timer), or a hardware context switch between virtual machine
>> and hypervisor. I know these is a tradeoff.
>>
>> Furthermore, here is the data we get when running benchmark contextswitch
>> to measure the latency(lower is better):
>>
>> 1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>>    3402.9 ns/ctxsw -- 199.8 %CPU
>>
>> 2. w/ patch and disable kvm dynamic poll:
>>    1163.5 ns/ctxsw -- 205.5 %CPU
>>
>> 3. w/ kvm dynamic poll:
>>    2280.6 ns/ctxsw -- 199.5 %CPU
>>
>> so, these tow solution are quite similar, but not duplicate..
>>
>> that's also why to add a generic idle poll before enter real idle path.
>> When a reschedule event is pending, we can bypass the real idle path.
>>
> There is a similar logic in the idle governor/driver, so how this
> patchset influence the decision in the idle governor/driver when
> running on bare-metal(power managment is not exposed to the guest so
> we will not enter into idle driver in the guest)?
>

This is expected to take effect only when running as a virtual machine with
proper CONFIG_* enabled. This can not work on bare mental even with proper
CONFIG_* enabled.

Quan
Alibaba Cloud

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Juergen Gross]

On 14/11/17 08:02, Quan Xu wrote:
> 
> 
> On 2017/11/13 18:53, Juergen Gross wrote:
>> On 13/11/17 11:06, Quan Xu wrote:
>>> From: Quan Xu <quan.xu0@gmail.com>
>>>
>>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
>>> in idle path which will poll for a while before we enter the real idle
>>> state.
>>>
>>> In virtualization, idle path includes several heavy operations
>>> includes timer access(LAPIC timer or TSC deadline timer) which will
>>> hurt performance especially for latency intensive workload like message
>>> passing task. The cost is mainly from the vmexit which is a hardware
>>> context switch between virtual machine and hypervisor. Our solution is
>>> to poll for a while and do not enter real idle path if we can get the
>>> schedule event during polling.
>>>
>>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>>> reduce the useless poll.
>>>
>>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>>> Cc: Juergen Gross <jgross@suse.com>
>>> Cc: Alok Kataria <akataria@vmware.com>
>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>> Cc: x86@kernel.org
>>> Cc: virtualization@lists.linux-foundation.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: xen-devel@lists.xenproject.org
>> Hmm, is the idle entry path really so critical to performance that a new
>> pvops function is necessary?
> Juergen, Here is the data we get when running benchmark netperf:
>  1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>     29031.6 bit/s -- 76.1 %CPU
> 
>  2. w/ patch and disable kvm dynamic poll (halt_poll_ns=0):
>     35787.7 bit/s -- 129.4 %CPU
> 
>  3. w/ kvm dynamic poll:
>     35735.6 bit/s -- 200.0 %CPU
> 
>  4. w/patch and w/ kvm dynamic poll:
>     42225.3 bit/s -- 198.7 %CPU
> 
>  5. idle=poll
>     37081.7 bit/s -- 998.1 %CPU
> 
> 
> 
>  w/ this patch, we will improve performance by 23%.. even we could improve
>  performance by 45.4%, if we use w/patch and w/ kvm dynamic poll. also the
>  cost of CPU is much lower than 'idle=poll' case..

I don't question the general idea. I just think pvops isn't the best way
to implement it.

>> Wouldn't a function pointer, maybe guarded
>> by a static key, be enough? A further advantage would be that this would
>> work on other architectures, too.
> 
> I assume this feature will be ported to other archs.. a new pvops makes
> code
> clean and easy to maintain. also I tried to add it into existed pvops,
> but it
> doesn't match.

You are aware that pvops is x86 only?

I really don't see the big difference in maintainability compared to the
static key / function pointer variant:

void (*guest_idle_poll_func)(void);
struct static_key guest_idle_poll_key __read_mostly;

static inline void guest_idle_poll(void)
{
	if (static_key_false(&guest_idle_poll_key))
		guest_idle_poll_func();
}

And KVM would just need to set guest_idle_poll_func and enable the
static key. Works on non-x86 architectures, too.


Juergen

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Quan Xu]

On 2017/11/14 15:30, Juergen Gross wrote:
> On 14/11/17 08:02, Quan Xu wrote:
>>
>> On 2017/11/13 18:53, Juergen Gross wrote:
>>> On 13/11/17 11:06, Quan Xu wrote:
>>>> From: Quan Xu <quan.xu0@gmail.com>
>>>>
>>>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
>>>> in idle path which will poll for a while before we enter the real idle
>>>> state.
>>>>
>>>> In virtualization, idle path includes several heavy operations
>>>> includes timer access(LAPIC timer or TSC deadline timer) which will
>>>> hurt performance especially for latency intensive workload like message
>>>> passing task. The cost is mainly from the vmexit which is a hardware
>>>> context switch between virtual machine and hypervisor. Our solution is
>>>> to poll for a while and do not enter real idle path if we can get the
>>>> schedule event during polling.
>>>>
>>>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>>>> reduce the useless poll.
>>>>
>>>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>>>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>>>> Cc: Juergen Gross <jgross@suse.com>
>>>> Cc: Alok Kataria <akataria@vmware.com>
>>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>> Cc: Ingo Molnar <mingo@redhat.com>
>>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>>> Cc: x86@kernel.org
>>>> Cc: virtualization@lists.linux-foundation.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Cc: xen-devel@lists.xenproject.org
>>> Hmm, is the idle entry path really so critical to performance that a new
>>> pvops function is necessary?
>> Juergen, Here is the data we get when running benchmark netperf:
>>   1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>>      29031.6 bit/s -- 76.1 %CPU
>>
>>   2. w/ patch and disable kvm dynamic poll (halt_poll_ns=0):
>>      35787.7 bit/s -- 129.4 %CPU
>>
>>   3. w/ kvm dynamic poll:
>>      35735.6 bit/s -- 200.0 %CPU
>>
>>   4. w/patch and w/ kvm dynamic poll:
>>      42225.3 bit/s -- 198.7 %CPU
>>
>>   5. idle=poll
>>      37081.7 bit/s -- 998.1 %CPU
>>
>>
>>
>>   w/ this patch, we will improve performance by 23%.. even we could improve
>>   performance by 45.4%, if we use w/patch and w/ kvm dynamic poll. also the
>>   cost of CPU is much lower than 'idle=poll' case..
> I don't question the general idea. I just think pvops isn't the best way
> to implement it.
>
>>> Wouldn't a function pointer, maybe guarded
>>> by a static key, be enough? A further advantage would be that this would
>>> work on other architectures, too.
>> I assume this feature will be ported to other archs.. a new pvops makes

       sorry, a typo.. /other archs/other hypervisors/
       it refers hypervisor like Xen, HyperV and VMware)..

>> code
>> clean and easy to maintain. also I tried to add it into existed pvops,
>> but it
>> doesn't match.
> You are aware that pvops is x86 only?

yes, I'm aware..

> I really don't see the big difference in maintainability compared to the
> static key / function pointer variant:
>
> void (*guest_idle_poll_func)(void);
> struct static_key guest_idle_poll_key __read_mostly;
>
> static inline void guest_idle_poll(void)
> {
> 	if (static_key_false(&guest_idle_poll_key))
> 		guest_idle_poll_func();
> }



thank you for your sample code :)
I agree there is no big difference.. I think we are discussion for two 
things:
  1) x86 VM on different hypervisors
  2) different archs VM on kvm hypervisor

What I want to do is x86 VM on different hypervisors, such as kvm / xen 
/ hyperv ..

> And KVM would just need to set guest_idle_poll_func and enable the
> static key. Works on non-x86 architectures, too.
>

.. referred to 'pv_mmu_ops', HyperV and Xen can implement their own 
functions for 'pv_mmu_ops'.
I think it is the same to pv_idle_ops.

with above explaination, do you still think I need to define the static
key/function pointer variant?

btw, any interest to port it to Xen HVM guest? :)

Quan
Alibaba Cloud

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Juergen Gross]

On 14/11/17 10:38, Quan Xu wrote:
> 
> 
> On 2017/11/14 15:30, Juergen Gross wrote:
>> On 14/11/17 08:02, Quan Xu wrote:
>>>
>>> On 2017/11/13 18:53, Juergen Gross wrote:
>>>> On 13/11/17 11:06, Quan Xu wrote:
>>>>> From: Quan Xu <quan.xu0@gmail.com>
>>>>>
>>>>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
>>>>> in idle path which will poll for a while before we enter the real idle
>>>>> state.
>>>>>
>>>>> In virtualization, idle path includes several heavy operations
>>>>> includes timer access(LAPIC timer or TSC deadline timer) which will
>>>>> hurt performance especially for latency intensive workload like
>>>>> message
>>>>> passing task. The cost is mainly from the vmexit which is a hardware
>>>>> context switch between virtual machine and hypervisor. Our solution is
>>>>> to poll for a while and do not enter real idle path if we can get the
>>>>> schedule event during polling.
>>>>>
>>>>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>>>>> reduce the useless poll.
>>>>>
>>>>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>>>>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>>>>> Cc: Juergen Gross <jgross@suse.com>
>>>>> Cc: Alok Kataria <akataria@vmware.com>
>>>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>>> Cc: Ingo Molnar <mingo@redhat.com>
>>>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>>>> Cc: x86@kernel.org
>>>>> Cc: virtualization@lists.linux-foundation.org
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Cc: xen-devel@lists.xenproject.org
>>>> Hmm, is the idle entry path really so critical to performance that a
>>>> new
>>>> pvops function is necessary?
>>> Juergen, Here is the data we get when running benchmark netperf:
>>>   1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>>>      29031.6 bit/s -- 76.1 %CPU
>>>
>>>   2. w/ patch and disable kvm dynamic poll (halt_poll_ns=0):
>>>      35787.7 bit/s -- 129.4 %CPU
>>>
>>>   3. w/ kvm dynamic poll:
>>>      35735.6 bit/s -- 200.0 %CPU
>>>
>>>   4. w/patch and w/ kvm dynamic poll:
>>>      42225.3 bit/s -- 198.7 %CPU
>>>
>>>   5. idle=poll
>>>      37081.7 bit/s -- 998.1 %CPU
>>>
>>>
>>>
>>>   w/ this patch, we will improve performance by 23%.. even we could
>>> improve
>>>   performance by 45.4%, if we use w/patch and w/ kvm dynamic poll.
>>> also the
>>>   cost of CPU is much lower than 'idle=poll' case..
>> I don't question the general idea. I just think pvops isn't the best way
>> to implement it.
>>
>>>> Wouldn't a function pointer, maybe guarded
>>>> by a static key, be enough? A further advantage would be that this
>>>> would
>>>> work on other architectures, too.
>>> I assume this feature will be ported to other archs.. a new pvops makes
> 
>       sorry, a typo.. /other archs/other hypervisors/
>       it refers hypervisor like Xen, HyperV and VMware)..
> 
>>> code
>>> clean and easy to maintain. also I tried to add it into existed pvops,
>>> but it
>>> doesn't match.
>> You are aware that pvops is x86 only?
> 
> yes, I'm aware..
> 
>> I really don't see the big difference in maintainability compared to the
>> static key / function pointer variant:
>>
>> void (*guest_idle_poll_func)(void);
>> struct static_key guest_idle_poll_key __read_mostly;
>>
>> static inline void guest_idle_poll(void)
>> {
>>     if (static_key_false(&guest_idle_poll_key))
>>         guest_idle_poll_func();
>> }
> 
> 
> 
> thank you for your sample code :)
> I agree there is no big difference.. I think we are discussion for two
> things:
>  1) x86 VM on different hypervisors
>  2) different archs VM on kvm hypervisor
> 
> What I want to do is x86 VM on different hypervisors, such as kvm / xen
> / hyperv ..

Why limit the solution to x86 if the more general solution isn't
harder?

As you didn't give any reason why the pvops approach is better other
than you don't care for non-x86 platforms you won't get an "Ack" from
me for this patch.

> 
>> And KVM would just need to set guest_idle_poll_func and enable the
>> static key. Works on non-x86 architectures, too.
>>
> 
> .. referred to 'pv_mmu_ops', HyperV and Xen can implement their own
> functions for 'pv_mmu_ops'.
> I think it is the same to pv_idle_ops.
> 
> with above explaination, do you still think I need to define the static
> key/function pointer variant?
> 
> btw, any interest to port it to Xen HVM guest? :)

Maybe. But this should work for Xen on ARM, too.


Juergen

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Quan Xu]

On 2017/11/14 18:27, Juergen Gross wrote:
> On 14/11/17 10:38, Quan Xu wrote:
>>
>> On 2017/11/14 15:30, Juergen Gross wrote:
>>> On 14/11/17 08:02, Quan Xu wrote:
>>>> On 2017/11/13 18:53, Juergen Gross wrote:
>>>>> On 13/11/17 11:06, Quan Xu wrote:
>>>>>> From: Quan Xu <quan.xu0@gmail.com>
>>>>>>
>>>>>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called
>>>>>> in idle path which will poll for a while before we enter the real idle
>>>>>> state.
>>>>>>
>>>>>> In virtualization, idle path includes several heavy operations
>>>>>> includes timer access(LAPIC timer or TSC deadline timer) which will
>>>>>> hurt performance especially for latency intensive workload like
>>>>>> message
>>>>>> passing task. The cost is mainly from the vmexit which is a hardware
>>>>>> context switch between virtual machine and hypervisor. Our solution is
>>>>>> to poll for a while and do not enter real idle path if we can get the
>>>>>> schedule event during polling.
>>>>>>
>>>>>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>>>>>> reduce the useless poll.
>>>>>>
>>>>>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>>>>>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>>>>>> Cc: Juergen Gross <jgross@suse.com>
>>>>>> Cc: Alok Kataria <akataria@vmware.com>
>>>>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>>>> Cc: Ingo Molnar <mingo@redhat.com>
>>>>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>>>>> Cc: x86@kernel.org
>>>>>> Cc: virtualization@lists.linux-foundation.org
>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>> Cc: xen-devel@lists.xenproject.org
>>>>> Hmm, is the idle entry path really so critical to performance that a
>>>>> new
>>>>> pvops function is necessary?
>>>> Juergen, Here is the data we get when running benchmark netperf:
>>>>    1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>>>>       29031.6 bit/s -- 76.1 %CPU
>>>>
>>>>    2. w/ patch and disable kvm dynamic poll (halt_poll_ns=0):
>>>>       35787.7 bit/s -- 129.4 %CPU
>>>>
>>>>    3. w/ kvm dynamic poll:
>>>>       35735.6 bit/s -- 200.0 %CPU
>>>>
>>>>    4. w/patch and w/ kvm dynamic poll:
>>>>       42225.3 bit/s -- 198.7 %CPU
>>>>
>>>>    5. idle=poll
>>>>       37081.7 bit/s -- 998.1 %CPU
>>>>
>>>>
>>>>
>>>>    w/ this patch, we will improve performance by 23%.. even we could
>>>> improve
>>>>    performance by 45.4%, if we use w/patch and w/ kvm dynamic poll.
>>>> also the
>>>>    cost of CPU is much lower than 'idle=poll' case..
>>> I don't question the general idea. I just think pvops isn't the best way
>>> to implement it.
>>>
>>>>> Wouldn't a function pointer, maybe guarded
>>>>> by a static key, be enough? A further advantage would be that this
>>>>> would
>>>>> work on other architectures, too.
>>>> I assume this feature will be ported to other archs.. a new pvops makes
>>        sorry, a typo.. /other archs/other hypervisors/
>>        it refers hypervisor like Xen, HyperV and VMware)..
>>
>>>> code
>>>> clean and easy to maintain. also I tried to add it into existed pvops,
>>>> but it
>>>> doesn't match.
>>> You are aware that pvops is x86 only?
>> yes, I'm aware..
>>
>>> I really don't see the big difference in maintainability compared to the
>>> static key / function pointer variant:
>>>
>>> void (*guest_idle_poll_func)(void);
>>> struct static_key guest_idle_poll_key __read_mostly;
>>>
>>> static inline void guest_idle_poll(void)
>>> {
>>>      if (static_key_false(&guest_idle_poll_key))
>>>          guest_idle_poll_func();
>>> }
>>
>>
>> thank you for your sample code :)
>> I agree there is no big difference.. I think we are discussion for two
>> things:
>>   1) x86 VM on different hypervisors
>>   2) different archs VM on kvm hypervisor
>>
>> What I want to do is x86 VM on different hypervisors, such as kvm / xen
>> / hyperv ..
> Why limit the solution to x86 if the more general solution isn't
> harder?
>
> As you didn't give any reason why the pvops approach is better other
> than you don't care for non-x86 platforms you won't get an "Ack" from
> me for this patch.


It just looks a little odder to me. I understand you care about no-x86 arch.

Are you aware 'pv_time_ops' for arm64/arm/x86 archs, defined in
    - arch/arm64/include/asm/paravirt.h
    - arch/x86/include/asm/paravirt_types.h
    - arch/arm/include/asm/paravirt.h

I am unfamilar with arm code. IIUC, if you'd implement pv_idle_ops
for arm/arm64 arch, you'd define a same structure in
    - arch/arm64/include/asm/paravirt.h     or
    - arch/arm/include/asm/paravirt.h

.. instead of static key / fuction.

then implement a real function in
    - arch/arm/kernel/paravirt.c.

Also I wonder HOW/WHERE to define a static key/function, then to benifit
x86/no-x86 archs?

Quan
Alibaba Cloud

>>> And KVM would just need to set guest_idle_poll_func and enable the
>>> static key. Works on non-x86 architectures, too.
>>>
>> .. referred to 'pv_mmu_ops', HyperV and Xen can implement their own
>> functions for 'pv_mmu_ops'.
>> I think it is the same to pv_idle_ops.
>>
>> with above explaination, do you still think I need to define the static
>> key/function pointer variant?
>>
>> btw, any interest to port it to Xen HVM guest? :)
> Maybe. But this should work for Xen on ARM, too.
>
>
> Juergen
>

Re: [PATCH RFC v3 1/6] x86/paravirt: Add pv_idle_ops to paravirt ops

[Juergen Gross]

On 14/11/17 12:43, Quan Xu wrote:
> 
> 
> On 2017/11/14 18:27, Juergen Gross wrote:
>> On 14/11/17 10:38, Quan Xu wrote:
>>>
>>> On 2017/11/14 15:30, Juergen Gross wrote:
>>>> On 14/11/17 08:02, Quan Xu wrote:
>>>>> On 2017/11/13 18:53, Juergen Gross wrote:
>>>>>> On 13/11/17 11:06, Quan Xu wrote:
>>>>>>> From: Quan Xu <quan.xu0@gmail.com>
>>>>>>>
>>>>>>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is
>>>>>>> called
>>>>>>> in idle path which will poll for a while before we enter the real
>>>>>>> idle
>>>>>>> state.
>>>>>>>
>>>>>>> In virtualization, idle path includes several heavy operations
>>>>>>> includes timer access(LAPIC timer or TSC deadline timer) which will
>>>>>>> hurt performance especially for latency intensive workload like
>>>>>>> message
>>>>>>> passing task. The cost is mainly from the vmexit which is a hardware
>>>>>>> context switch between virtual machine and hypervisor. Our
>>>>>>> solution is
>>>>>>> to poll for a while and do not enter real idle path if we can get
>>>>>>> the
>>>>>>> schedule event during polling.
>>>>>>>
>>>>>>> Poll may cause the CPU waste so we adopt a smart polling
>>>>>>> mechanism to
>>>>>>> reduce the useless poll.
>>>>>>>
>>>>>>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com>
>>>>>>> Signed-off-by: Quan Xu <quan.xu0@gmail.com>
>>>>>>> Cc: Juergen Gross <jgross@suse.com>
>>>>>>> Cc: Alok Kataria <akataria@vmware.com>
>>>>>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>>>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>>>>> Cc: Ingo Molnar <mingo@redhat.com>
>>>>>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>>>>>> Cc: x86@kernel.org
>>>>>>> Cc: virtualization@lists.linux-foundation.org
>>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>>> Cc: xen-devel@lists.xenproject.org
>>>>>> Hmm, is the idle entry path really so critical to performance that a
>>>>>> new
>>>>>> pvops function is necessary?
>>>>> Juergen, Here is the data we get when running benchmark netperf:
>>>>>    1. w/o patch and disable kvm dynamic poll (halt_poll_ns=0):
>>>>>       29031.6 bit/s -- 76.1 %CPU
>>>>>
>>>>>    2. w/ patch and disable kvm dynamic poll (halt_poll_ns=0):
>>>>>       35787.7 bit/s -- 129.4 %CPU
>>>>>
>>>>>    3. w/ kvm dynamic poll:
>>>>>       35735.6 bit/s -- 200.0 %CPU
>>>>>
>>>>>    4. w/patch and w/ kvm dynamic poll:
>>>>>       42225.3 bit/s -- 198.7 %CPU
>>>>>
>>>>>    5. idle=poll
>>>>>       37081.7 bit/s -- 998.1 %CPU
>>>>>
>>>>>
>>>>>
>>>>>    w/ this patch, we will improve performance by 23%.. even we could
>>>>> improve
>>>>>    performance by 45.4%, if we use w/patch and w/ kvm dynamic poll.
>>>>> also the
>>>>>    cost of CPU is much lower than 'idle=poll' case..
>>>> I don't question the general idea. I just think pvops isn't the best
>>>> way
>>>> to implement it.
>>>>
>>>>>> Wouldn't a function pointer, maybe guarded
>>>>>> by a static key, be enough? A further advantage would be that this
>>>>>> would
>>>>>> work on other architectures, too.
>>>>> I assume this feature will be ported to other archs.. a new pvops
>>>>> makes
>>>        sorry, a typo.. /other archs/other hypervisors/
>>>        it refers hypervisor like Xen, HyperV and VMware)..
>>>
>>>>> code
>>>>> clean and easy to maintain. also I tried to add it into existed pvops,
>>>>> but it
>>>>> doesn't match.
>>>> You are aware that pvops is x86 only?
>>> yes, I'm aware..
>>>
>>>> I really don't see the big difference in maintainability compared to
>>>> the
>>>> static key / function pointer variant:
>>>>
>>>> void (*guest_idle_poll_func)(void);
>>>> struct static_key guest_idle_poll_key __read_mostly;
>>>>
>>>> static inline void guest_idle_poll(void)
>>>> {
>>>>      if (static_key_false(&guest_idle_poll_key))
>>>>          guest_idle_poll_func();
>>>> }
>>>
>>>
>>> thank you for your sample code :)
>>> I agree there is no big difference.. I think we are discussion for two
>>> things:
>>>   1) x86 VM on different hypervisors
>>>   2) different archs VM on kvm hypervisor
>>>
>>> What I want to do is x86 VM on different hypervisors, such as kvm / xen
>>> / hyperv ..
>> Why limit the solution to x86 if the more general solution isn't
>> harder?
>>
>> As you didn't give any reason why the pvops approach is better other
>> than you don't care for non-x86 platforms you won't get an "Ack" from
>> me for this patch.
> 
> 
> It just looks a little odder to me. I understand you care about no-x86
> arch.
> 
> Are you aware 'pv_time_ops' for arm64/arm/x86 archs, defined in
>    - arch/arm64/include/asm/paravirt.h
>    - arch/x86/include/asm/paravirt_types.h
>    - arch/arm/include/asm/paravirt.h

Yes, I know. This is just a hack to make it compile. Other than the
same names this has nothing to do with pvops, but is just a function
vector.

> I am unfamilar with arm code. IIUC, if you'd implement pv_idle_ops
> for arm/arm64 arch, you'd define a same structure in
>    - arch/arm64/include/asm/paravirt.h     or
>    - arch/arm/include/asm/paravirt.h
> 
> .. instead of static key / fuction.
> 
> then implement a real function in
>    - arch/arm/kernel/paravirt.c.

So just to use pvops you want to implement it in each arch instead
of using a mechanism available everywhere?

> Also I wonder HOW/WHERE to define a static key/function, then to benifit
> x86/no-x86 archs?

What? There are plenty of examples in the kernel.

Please stop wasting my time. Either write a patch which is acceptable
or let it be. I won't take your pvops approach without a really good
reason to do so. And so far you haven't given any reason other than
you are too lazy to write a proper patch, sorry.


Juergen

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Peter Zijlstra]

On Mon, Nov 13, 2017 at 06:06:02PM +0800, Quan Xu wrote:
> From: Yang Zhang <yang.zhang.wz@gmail.com>
> 
> Implement a generic idle poll which resembles the functionality
> found in arch/. Provide weak arch_cpu_idle_poll function which
> can be overridden by the architecture code if needed.

No, we want less of those magic hooks, not more.

> Interrupts arrive which may not cause a reschedule in idle loops.
> In KVM guest, this costs several VM-exit/VM-entry cycles, VM-entry
> for interrupts and VM-exit immediately. Also this becomes more
> expensive than bare metal. Add a generic idle poll before enter
> real idle path. When a reschedule event is pending, we can bypass
> the real idle path.

Why not do a HV specific idle driver?

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Thomas Gleixner]

On Wed, 15 Nov 2017, Peter Zijlstra wrote:

> On Mon, Nov 13, 2017 at 06:06:02PM +0800, Quan Xu wrote:
> > From: Yang Zhang <yang.zhang.wz@gmail.com>
> > 
> > Implement a generic idle poll which resembles the functionality
> > found in arch/. Provide weak arch_cpu_idle_poll function which
> > can be overridden by the architecture code if needed.
> 
> No, we want less of those magic hooks, not more.
> 
> > Interrupts arrive which may not cause a reschedule in idle loops.
> > In KVM guest, this costs several VM-exit/VM-entry cycles, VM-entry
> > for interrupts and VM-exit immediately. Also this becomes more
> > expensive than bare metal. Add a generic idle poll before enter
> > real idle path. When a reschedule event is pending, we can bypass
> > the real idle path.
> 
> Why not do a HV specific idle driver?

If I understand the problem correctly then he wants to avoid the heavy
lifting in tick_nohz_idle_enter() in the first place, but there is already
an interesting quirk there which makes it exit early.  See commit
3c5d92a0cfb5 ("nohz: Introduce arch_needs_cpu"). The reason for this commit
looks similar. But lets not proliferate that. I'd rather see that go away.

But the irq_timings stuff is heading into the same direction, with a more
complex prediction logic which should tell you pretty good how long that
idle period is going to be and in case of an interrupt heavy workload this
would skip the extra work of stopping and restarting the tick and provide a
very good input into a polling decision.

This can be handled either in a HV specific idle driver or even in the
generic core code. If the interrupt does not arrive then you can assume
within the predicted time then you can assume that the flood stopped and
invoke halt or whatever.

That avoids all of that 'tunable and tweakable' x86 specific hackery and
utilizes common functionality which is mostly there already.

Thanks,

	tglx

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Peter Zijlstra]

On Wed, Nov 15, 2017 at 11:03:08PM +0100, Thomas Gleixner wrote:
> If I understand the problem correctly then he wants to avoid the heavy
> lifting in tick_nohz_idle_enter() in the first place, but there is already
> an interesting quirk there which makes it exit early. 

Sure. And there are people who want to do the same for native.

Adding more ugly and special cases just isn't the way to go about doing
that.

I'm fairly sure I've told the various groups that want to tinker with
this to work together on this. I've also in fairly significant detail
sketched how to rework the idle code and idle predictors.

At this point I'm too tired to dig any of that up, so I'll just keep
saying no to patches that don't even attempt to go in the right
direction.

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Thomas Gleixner]

On Thu, 16 Nov 2017, Peter Zijlstra wrote:

> On Wed, Nov 15, 2017 at 11:03:08PM +0100, Thomas Gleixner wrote:
> > If I understand the problem correctly then he wants to avoid the heavy
> > lifting in tick_nohz_idle_enter() in the first place, but there is already
> > an interesting quirk there which makes it exit early. 
> 
> Sure. And there are people who want to do the same for native.
> 
> Adding more ugly and special cases just isn't the way to go about doing
> that.
> 
> I'm fairly sure I've told the various groups that want to tinker with
> this to work together on this. I've also in fairly significant detail
> sketched how to rework the idle code and idle predictors.
> 
> At this point I'm too tired to dig any of that up, so I'll just keep
> saying no to patches that don't even attempt to go in the right
> direction.

That's why I said: But lets not proliferate that. I'd rather see that go
away.

And yes, the VM folks should talk to those who are trying to solve similar
problems for native (embedded/mobile).

Thanks,

	tglx

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Quan Xu]

On 2017-11-16 16:45, Peter Zijlstra wrote:
> On Wed, Nov 15, 2017 at 11:03:08PM +0100, Thomas Gleixner wrote:
>> If I understand the problem correctly then he wants to avoid the heavy
>> lifting in tick_nohz_idle_enter() in the first place, but there is already
>> an interesting quirk there which makes it exit early.
> Sure. And there are people who want to do the same for native.
>
> Adding more ugly and special cases just isn't the way to go about doing
> that.
>
> I'm fairly sure I've told the various groups that want to tinker with
> this to work together on this. I've also in fairly significant detail
> sketched how to rework the idle code and idle predictors.
>
> At this point I'm too tired to dig any of that up, so I'll just keep
> saying no to patches that don't even attempt to go in the right
> direction.
Peter, take care.

I really have considered this factor, and try my best not to interfere 
with scheduler/idle code.
if irq_timings code is ready, I can use it directly. I think irq_timings 
is not an easy task, I'd
like to help as much as I can.  Also don't try to touch tick_nohz* code 
again.

as tglx suggested, this can be handled either in a HV specific idle driver or even in the generic core code.

I hope this is in the right direction.

Quan
Alibaba Cloud

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Thomas Gleixner]

On Thu, 16 Nov 2017, Quan Xu wrote:
> On 2017-11-16 16:45, Peter Zijlstra wrote:
> 
> I really have considered this factor, and try my best not to interfere with
> scheduler/idle code.
>
> if irq_timings code is ready, I can use it directly. I think irq_timings
> is not an easy task, I'd like to help as much as I can.

It's not a question whether irq_timings code is ready or not.

The infrastructure is there. I said that before and I'm happy to repeat:

 All parties who need this kind of prediction should:

     1) Talk to each other

     2) Work together to make it usable for _ALL_ use cases

 AFAICT, that never happened. Otherwise there would be either progress on
 that or at least a reasonable explanation WHY it cannot be done.

Peter and myself made it entirely clear several times in the past that this
must be solved at the generic level without any magic hackery in random
places. But the only thing we've seen so far is the magic hackery coming
around in a different flavour some time after we rejected the last one.

We can play that game forever. The outcome is extremly predictable.

Thanks,

	tglx

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Quan Xu]

On 2017-11-16 06:03, Thomas Gleixner wrote:
> On Wed, 15 Nov 2017, Peter Zijlstra wrote:
>
>> On Mon, Nov 13, 2017 at 06:06:02PM +0800, Quan Xu wrote:
>>> From: Yang Zhang <yang.zhang.wz@gmail.com>
>>>
>>> Implement a generic idle poll which resembles the functionality
>>> found in arch/. Provide weak arch_cpu_idle_poll function which
>>> can be overridden by the architecture code if needed.
>> No, we want less of those magic hooks, not more.
>>
>>> Interrupts arrive which may not cause a reschedule in idle loops.
>>> In KVM guest, this costs several VM-exit/VM-entry cycles, VM-entry
>>> for interrupts and VM-exit immediately. Also this becomes more
>>> expensive than bare metal. Add a generic idle poll before enter
>>> real idle path. When a reschedule event is pending, we can bypass
>>> the real idle path.
>> Why not do a HV specific idle driver?
> If I understand the problem correctly then he wants to avoid the heavy
> lifting in tick_nohz_idle_enter() in the first place, but there is already
> an interesting quirk there which makes it exit early.  See commit
> 3c5d92a0cfb5 ("nohz: Introduce arch_needs_cpu"). The reason for this commit
> looks similar. But lets not proliferate that. I'd rather see that go away.

agreed.

Even we can get more benifit than commit 3c5d92a0cfb5 ("nohz: Introduce 
arch_needs_cpu")
in kvm guest. I won't proliferate that..

> But the irq_timings stuff is heading into the same direction, with a more
> complex prediction logic which should tell you pretty good how long that
> idle period is going to be and in case of an interrupt heavy workload this
> would skip the extra work of stopping and restarting the tick and provide a
> very good input into a polling decision.


interesting. I have tested with IRQ_TIMINGS related code, which seems 
not working so far.
Also I'd like to help as much as I can.
> This can be handled either in a HV specific idle driver or even in the
> generic core code. If the interrupt does not arrive then you can assume
> within the predicted time then you can assume that the flood stopped and
> invoke halt or whatever.
>
> That avoids all of that 'tunable and tweakable' x86 specific hackery and
> utilizes common functionality which is mostly there already.
here is some sample code. Poll for a while before enter halt in 
cpuidle_enter_state()
If I get a reschedule event, then don't try to enter halt.  (I hope this 
is the right direction as Peter mentioned in another email)

--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -210,6 +210,13 @@ int cpuidle_enter_state(struct cpuidle_device *dev, 
struct cpuidle_driver *drv,
                 target_state = &drv->states[index];
         }

+#ifdef CONFIG_PARAVIRT
+       paravirt_idle_poll();
+
+       if (need_resched())
+               return -EBUSY;
+#endif
+
         /* Take note of the planned idle state. */
         sched_idle_set_state(target_state);




thanks,

Quan
Alibaba Cloud

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Daniel Lezcano]

On 16/11/2017 10:12, Quan Xu wrote:
> 
> 
> On 2017-11-16 06:03, Thomas Gleixner wrote:
>> On Wed, 15 Nov 2017, Peter Zijlstra wrote:
>>
>>> On Mon, Nov 13, 2017 at 06:06:02PM +0800, Quan Xu wrote:
>>>> From: Yang Zhang <yang.zhang.wz@gmail.com>
>>>>
>>>> Implement a generic idle poll which resembles the functionality
>>>> found in arch/. Provide weak arch_cpu_idle_poll function which
>>>> can be overridden by the architecture code if needed.
>>> No, we want less of those magic hooks, not more.
>>>
>>>> Interrupts arrive which may not cause a reschedule in idle loops.
>>>> In KVM guest, this costs several VM-exit/VM-entry cycles, VM-entry
>>>> for interrupts and VM-exit immediately. Also this becomes more
>>>> expensive than bare metal. Add a generic idle poll before enter
>>>> real idle path. When a reschedule event is pending, we can bypass
>>>> the real idle path.
>>> Why not do a HV specific idle driver?
>> If I understand the problem correctly then he wants to avoid the heavy
>> lifting in tick_nohz_idle_enter() in the first place, but there is
>> already
>> an interesting quirk there which makes it exit early.  See commit
>> 3c5d92a0cfb5 ("nohz: Introduce arch_needs_cpu"). The reason for this
>> commit
>> looks similar. But lets not proliferate that. I'd rather see that go
>> away.
> 
> agreed.
> 
> Even we can get more benifit than commit 3c5d92a0cfb5 ("nohz: Introduce
> arch_needs_cpu")
> in kvm guest. I won't proliferate that..
> 
>> But the irq_timings stuff is heading into the same direction, with a more
>> complex prediction logic which should tell you pretty good how long that
>> idle period is going to be and in case of an interrupt heavy workload
>> this
>> would skip the extra work of stopping and restarting the tick and
>> provide a
>> very good input into a polling decision.
> 
> 
> interesting. I have tested with IRQ_TIMINGS related code, which seems
> not working so far.

I don't know how you tested it, can you elaborate what you meant by
"seems not working so far" ?

There are still some work to do to be more efficient. The prediction
based on the irq timings is all right if the interrupts have a simple
periodicity. But as soon as there is a pattern, the current code can't
handle it properly and does bad predictions.

I'm working on a self-learning pattern detection which is too heavy for
the kernel, and with it we should be able to detect properly the
patterns and re-ajust the period if it changes. I'm in the process of
making it suitable for kernel code (both math and perf).

One improvement which can be done right now and which can help you is
the interrupts rate on the CPU. It is possible to compute it and that
will give an accurate information for the polling decision.



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Quan Xu]

On 2017-11-16 17:45, Daniel Lezcano wrote:
> On 16/11/2017 10:12, Quan Xu wrote:
>>
>> On 2017-11-16 06:03, Thomas Gleixner wrote:
>>> On Wed, 15 Nov 2017, Peter Zijlstra wrote:
>>>
>>>> On Mon, Nov 13, 2017 at 06:06:02PM +0800, Quan Xu wrote:
>>>>> From: Yang Zhang <yang.zhang.wz@gmail.com>
>>>>>
>>>>> Implement a generic idle poll which resembles the functionality
>>>>> found in arch/. Provide weak arch_cpu_idle_poll function which
>>>>> can be overridden by the architecture code if needed.
>>>> No, we want less of those magic hooks, not more.
>>>>
>>>>> Interrupts arrive which may not cause a reschedule in idle loops.
>>>>> In KVM guest, this costs several VM-exit/VM-entry cycles, VM-entry
>>>>> for interrupts and VM-exit immediately. Also this becomes more
>>>>> expensive than bare metal. Add a generic idle poll before enter
>>>>> real idle path. When a reschedule event is pending, we can bypass
>>>>> the real idle path.
>>>> Why not do a HV specific idle driver?
>>> If I understand the problem correctly then he wants to avoid the heavy
>>> lifting in tick_nohz_idle_enter() in the first place, but there is
>>> already
>>> an interesting quirk there which makes it exit early.  See commit
>>> 3c5d92a0cfb5 ("nohz: Introduce arch_needs_cpu"). The reason for this
>>> commit
>>> looks similar. But lets not proliferate that. I'd rather see that go
>>> away.
>> agreed.
>>
>> Even we can get more benifit than commit 3c5d92a0cfb5 ("nohz: Introduce
>> arch_needs_cpu")
>> in kvm guest. I won't proliferate that..
>>
>>> But the irq_timings stuff is heading into the same direction, with a more
>>> complex prediction logic which should tell you pretty good how long that
>>> idle period is going to be and in case of an interrupt heavy workload
>>> this
>>> would skip the extra work of stopping and restarting the tick and
>>> provide a
>>> very good input into a polling decision.
>>
>> interesting. I have tested with IRQ_TIMINGS related code, which seems
>> not working so far.
> I don't know how you tested it, can you elaborate what you meant by
> "seems not working so far" ?

Daniel, I tried to enable IRQ_TIMINGS* manually. used 
irq_timings_next_event()
to return estimation of the earliest interrupt. However I got a constant.

> There are still some work to do to be more efficient. The prediction
> based on the irq timings is all right if the interrupts have a simple
> periodicity. But as soon as there is a pattern, the current code can't
> handle it properly and does bad predictions.
>
> I'm working on a self-learning pattern detection which is too heavy for
> the kernel, and with it we should be able to detect properly the
> patterns and re-ajust the period if it changes. I'm in the process of
> making it suitable for kernel code (both math and perf).
>
> One improvement which can be done right now and which can help you is
> the interrupts rate on the CPU. It is possible to compute it and that
> will give an accurate information for the polling decision.
>
>
As tglx said, talk to each other / work together to make it usable for 
all use cases.
could you share how to enable it to get the interrupts rate on the CPU? 
I can try it
in cloud scenario. of course, I'd like to work with you to improve it.

Quan
Alibaba Cloud

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Daniel Lezcano]

On 20/11/2017 08:05, Quan Xu wrote:

[ ... ]

>>>> But the irq_timings stuff is heading into the same direction, with a
>>>> more
>>>> complex prediction logic which should tell you pretty good how long
>>>> that
>>>> idle period is going to be and in case of an interrupt heavy workload
>>>> this
>>>> would skip the extra work of stopping and restarting the tick and
>>>> provide a
>>>> very good input into a polling decision.
>>>
>>> interesting. I have tested with IRQ_TIMINGS related code, which seems
>>> not working so far.
>> I don't know how you tested it, can you elaborate what you meant by
>> "seems not working so far" ?
> 
> Daniel, I tried to enable IRQ_TIMINGS* manually. used
> irq_timings_next_event()
> to return estimation of the earliest interrupt. However I got a constant.

The irq timings gives you an indication of the next interrupt deadline.

This information is a piece of the puzzle, you need to combine it with
the next timer expiration, and the next scheduling event. Then take the
earliest event in a timeline basis.

Using the trivial scheme above will work well with workload like videos
or mp3 but will fail as soon as the interrupts are not coming in a
regular basis and this is where the pattern recognition algorithm must act.

>> There are still some work to do to be more efficient. The prediction
>> based on the irq timings is all right if the interrupts have a simple
>> periodicity. But as soon as there is a pattern, the current code can't
>> handle it properly and does bad predictions.
>>
>> I'm working on a self-learning pattern detection which is too heavy for
>> the kernel, and with it we should be able to detect properly the
>> patterns and re-ajust the period if it changes. I'm in the process of
>> making it suitable for kernel code (both math and perf).
>>
>> One improvement which can be done right now and which can help you is
>> the interrupts rate on the CPU. It is possible to compute it and that
>> will give an accurate information for the polling decision.
>>
>>
> As tglx said, talk to each other / work together to make it usable for
> all use cases.
> could you share how to enable it to get the interrupts rate on the CPU?
> I can try it
> in cloud scenario. of course, I'd like to work with you to improve it.

Sure, I will be glad if we can collaborate. I have some draft code but
before sharing it I would like we define what is the rate and what kind
of information we expect to infer from it. From my point of view it is a
value indicating the interrupt period per CPU, a short value indicates a
high number of interrupts on the CPU.

This value must decay with the time, the question here is what decay
function we apply to the rate from the last timestamp ?




-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Thomas Gleixner]

[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]

On Thu, 16 Nov 2017, Quan Xu wrote:
> On 2017-11-16 06:03, Thomas Gleixner wrote:
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -210,6 +210,13 @@ int cpuidle_enter_state(struct cpuidle_device *dev,
> struct cpuidle_driver *drv,
>                 target_state = &drv->states[index];
>         }
> 
> +#ifdef CONFIG_PARAVIRT
> +       paravirt_idle_poll();
> +
> +       if (need_resched())
> +               return -EBUSY;
> +#endif

That's just plain wrong. We don't want to see any of this PARAVIRT crap in
anything outside the architecture/hypervisor interfacing code which really
needs it.

The problem can and must be solved at the generic level in the first place
to gather the data which can be used to make such decisions.

How that information is used might be either completely generic or requires
system specific variants. But as long as we don't have any information at
all we cannot discuss that.

Please sit down and write up which data needs to be considered to make
decisions about probabilistic polling. Then we need to compare and contrast
that with the data which is necessary to make power/idle state decisions.

I would be very surprised if this data would not overlap by at least 90%.

Thanks,

	tglx

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Quan Xu]

On 2017-11-16 17:53, Thomas Gleixner wrote:
> On Thu, 16 Nov 2017, Quan Xu wrote:
>> On 2017-11-16 06:03, Thomas Gleixner wrote:
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -210,6 +210,13 @@ int cpuidle_enter_state(struct cpuidle_device *dev,
>> struct cpuidle_driver *drv,
>>                  target_state = &drv->states[index];
>>          }
>>
>> +#ifdef CONFIG_PARAVIRT
>> +       paravirt_idle_poll();
>> +
>> +       if (need_resched())
>> +               return -EBUSY;
>> +#endif
> That's just plain wrong. We don't want to see any of this PARAVIRT crap in
> anything outside the architecture/hypervisor interfacing code which really
> needs it.
>
> The problem can and must be solved at the generic level in the first place
> to gather the data which can be used to make such decisions.
>
> How that information is used might be either completely generic or requires
> system specific variants. But as long as we don't have any information at
> all we cannot discuss that.
>
> Please sit down and write up which data needs to be considered to make
> decisions about probabilistic polling. Then we need to compare and contrast
> that with the data which is necessary to make power/idle state decisions.
>
> I would be very surprised if this data would not overlap by at least 90%.
>

Peter, tglx
Thanks for your comments..

rethink of this patch set,

1. which data needs to considerd to make decisions about probabilistic 
polling

I really need to write up which data needs to considerd to make
decisions about probabilistic polling. At last several months,
I always focused on the data _from idle to reschedule_, then to bypass
the idle loops. unfortunately, this makes me touch scheduler/idle/nohz
code inevitably.

with tglx's suggestion, the data which is necessary to make power/idle
state decisions, is the last idle state's residency time. IIUC this data
is duration from idle to wakeup, which maybe by reschedule irq or other irq.

I also test that the reschedule irq overlap by more than 90% (trace the
need_resched status after cpuidle_idle_call), when I run ctxsw/netperf for
one minute.

as the overlap, I think I can input the last idle state's residency time
to make decisions about probabilistic polling, as @dev->last_residency does.
it is much easier to get data.


2. do a HV specific idle driver (function)

so far, power management is not exposed to guest.. idle is simple for 
KVM guest,
calling "sti" / "hlt"(cpuidle_idle_call() --> default_idle_call())..
thanks Xen guys, who has implemented the paravirt framework. I can 
implement it
as easy as following:

              --- a/arch/x86/kernel/kvm.c
              +++ b/arch/x86/kernel/kvm.c
              @@ -465,6 +465,12 @@ static void __init 
kvm_apf_trap_init(void)
                      update_intr_gate(X86_TRAP_PF, async_page_fault);
               }

              +static __cpuidle void kvm_safe_halt(void)
              +{
          +        /* 1. POLL, if need_resched() --> return */
          +
              +        asm volatile("sti; hlt": : :"memory"); /* 2. halt */
              +
          +        /* 3. get the last idle state's residency time */
              +
          +        /* 4. update poll duration based on last idle state's 
residency time */
              +}
              +
               void __init kvm_guest_init(void)
               {
                      int i;
              @@ -490,6 +496,8 @@ void __init kvm_guest_init(void)
                      if (kvmclock_vsyscall)
                              kvm_setup_vsyscall_timeinfo();

              +       pv_irq_ops.safe_halt = kvm_safe_halt;
              +
               #ifdef CONFIG_SMP




then, I am no need to introduce a new pvops, and never modify 
schedule/idle/nohz code again.
also I can narrow all of the code down in arch/x86/kernel/kvm.c.

If this is in the right direction, I will send a new patch set next week..

thanks,

Quan
Alibaba Cloud

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Thomas Gleixner]

[-- Attachment #1: Type: text/plain, Size: 3978 bytes --]

On Fri, 17 Nov 2017, Quan Xu wrote:
> On 2017-11-16 17:53, Thomas Gleixner wrote:
> > That's just plain wrong. We don't want to see any of this PARAVIRT crap in
> > anything outside the architecture/hypervisor interfacing code which really
> > needs it.
> > 
> > The problem can and must be solved at the generic level in the first place
> > to gather the data which can be used to make such decisions.
> > 
> > How that information is used might be either completely generic or requires
> > system specific variants. But as long as we don't have any information at
> > all we cannot discuss that.
> > 
> > Please sit down and write up which data needs to be considered to make
> > decisions about probabilistic polling. Then we need to compare and contrast
> > that with the data which is necessary to make power/idle state decisions.
> > 
> > I would be very surprised if this data would not overlap by at least 90%.
> > 
> 1. which data needs to considerd to make decisions about probabilistic polling
> 
> I really need to write up which data needs to considerd to make
> decisions about probabilistic polling. At last several months,
> I always focused on the data _from idle to reschedule_, then to bypass
> the idle loops. unfortunately, this makes me touch scheduler/idle/nohz
> code inevitably.
> 
> with tglx's suggestion, the data which is necessary to make power/idle
> state decisions, is the last idle state's residency time. IIUC this data
> is duration from idle to wakeup, which maybe by reschedule irq or other irq.

That's part of the picture, but not complete.

> I also test that the reschedule irq overlap by more than 90% (trace the
> need_resched status after cpuidle_idle_call), when I run ctxsw/netperf for
> one minute.
> 
> as the overlap, I think I can input the last idle state's residency time
> to make decisions about probabilistic polling, as @dev->last_residency does.
> it is much easier to get data.

That's only true for your particular use case.

> 
> 2. do a HV specific idle driver (function)
> 
> so far, power management is not exposed to guest.. idle is simple for KVM
> guest,
> calling "sti" / "hlt"(cpuidle_idle_call() --> default_idle_call())..
> thanks Xen guys, who has implemented the paravirt framework. I can implement
> it
> as easy as following:
> 
>              --- a/arch/x86/kernel/kvm.c

Your email client is using a very strange formatting. 

>              +++ b/arch/x86/kernel/kvm.c
>              @@ -465,6 +465,12 @@ static void __init kvm_apf_trap_init(void)
>                      update_intr_gate(X86_TRAP_PF, async_page_fault);
>               }
> 
>              +static __cpuidle void kvm_safe_halt(void)
>              +{
>          +        /* 1. POLL, if need_resched() --> return */
>          +
>              +        asm volatile("sti; hlt": : :"memory"); /* 2. halt */
>              +
>          +        /* 3. get the last idle state's residency time */
>              +
>          +        /* 4. update poll duration based on last idle state's
> residency time */
>              +}
>              +
>               void __init kvm_guest_init(void)
>               {
>                      int i;
>              @@ -490,6 +496,8 @@ void __init kvm_guest_init(void)
>                      if (kvmclock_vsyscall)
>                              kvm_setup_vsyscall_timeinfo();
> 
>              +       pv_irq_ops.safe_halt = kvm_safe_halt;
>              +
>               #ifdef CONFIG_SMP
> 
> 
> then, I am no need to introduce a new pvops, and never modify
> schedule/idle/nohz code again.
> also I can narrow all of the code down in arch/x86/kernel/kvm.c.
> 
> If this is in the right direction, I will send a new patch set next week..

This is definitely better than what you proposed so far and implementing it
as a prove of concept seems to be worthwhile.

But I doubt that this is the final solution. It's not generic and not
necessarily suitable for all use case scenarios.

Thanks,

	tglx

Re: [PATCH RFC v3 3/6] sched/idle: Add a generic poll before enter real idle path

[Quan Xu]

On 2017-11-17 19:36, Thomas Gleixner wrote:
> On Fri, 17 Nov 2017, Quan Xu wrote:
>> On 2017-11-16 17:53, Thomas Gleixner wrote:
>>> That's just plain wrong. We don't want to see any of this PARAVIRT crap in
>>> anything outside the architecture/hypervisor interfacing code which really
>>> needs it.
>>>
>>> The problem can and must be solved at the generic level in the first place
>>> to gather the data which can be used to make such decisions.
>>>
>>> How that information is used might be either completely generic or requires
>>> system specific variants. But as long as we don't have any information at
>>> all we cannot discuss that.
>>>
>>> Please sit down and write up which data needs to be considered to make
>>> decisions about probabilistic polling. Then we need to compare and contrast
>>> that with the data which is necessary to make power/idle state decisions.
>>>
>>> I would be very surprised if this data would not overlap by at least 90%.
>>>
>> 1. which data needs to considerd to make decisions about probabilistic polling
>>
>> I really need to write up which data needs to considerd to make
>> decisions about probabilistic polling. At last several months,
>> I always focused on the data _from idle to reschedule_, then to bypass
>> the idle loops. unfortunately, this makes me touch scheduler/idle/nohz
>> code inevitably.
>>
>> with tglx's suggestion, the data which is necessary to make power/idle
>> state decisions, is the last idle state's residency time. IIUC this data
>> is duration from idle to wakeup, which maybe by reschedule irq or other irq.
> That's part of the picture, but not complete.

tglx, could you share more? I am very curious about it..

>> I also test that the reschedule irq overlap by more than 90% (trace the
>> need_resched status after cpuidle_idle_call), when I run ctxsw/netperf for
>> one minute.
>>
>> as the overlap, I think I can input the last idle state's residency time
>> to make decisions about probabilistic polling, as @dev->last_residency does.
>> it is much easier to get data.
> That's only true for your particular use case.
>
>> 2. do a HV specific idle driver (function)
>>
>> so far, power management is not exposed to guest.. idle is simple for KVM
>> guest,
>> calling "sti" / "hlt"(cpuidle_idle_call() --> default_idle_call())..
>> thanks Xen guys, who has implemented the paravirt framework. I can implement
>> it
>> as easy as following:
>>
>>               --- a/arch/x86/kernel/kvm.c
> Your email client is using a very strange formatting.

my bad, I insert space to highlight these code.

> This is definitely better than what you proposed so far and implementing it
> as a prove of concept seems to be worthwhile.
>
> But I doubt that this is the final solution. It's not generic and not
> necessarily suitable for all use case scenarios.
>
>
yes, I am exhausted :):)


could you tell me the gap to be generic and necessarily suitable for
all use case scenarios? as lack of irq/idle predictors?

  I really want to upstream it for all of public cloud users/providers..

as kvm host has a similar one, is it possible to upstream with following 
conditions? :
     1). add a QEMU configuration, whether enable or not, by default 
disable.
     2). add some "TODO" comments near the code.
     3). ...


anyway, thanks for your help..

Quan
  Alibaba Cloud

Re: [Xen-devel] [PATCH RFC v3 0/6] x86/idle: add halt poll support

[Konrad Rzeszutek Wilk]

On Mon, Nov 13, 2017 at 06:05:59PM +0800, Quan Xu wrote:
> From: Yang Zhang <yang.zhang.wz@gmail.com>
> 
> Some latency-intensive workload have seen obviously performance
> drop when running inside VM. The main reason is that the overhead
> is amplified when running inside VM. The most cost I have seen is
> inside idle path.

Meaning an VMEXIT b/c it is an 'halt' operation ? And then going
back in guest (VMRESUME) takes time. And hence your latency gets
all whacked b/c of this?

So if I understand - you want to use your _full_ timeslice (of the guest)
without ever (or as much as possible) to go in the hypervisor?

Which means in effect you don't care about power-saving or CPUfreq
savings, you just want to eat the full CPU for snack?

> 
> This patch introduces a new mechanism to poll for a while before
> entering idle state. If schedule is needed during poll, then we
> don't need to goes through the heavy overhead path.

Schedule of what? The guest or the host?

Re: [Xen-devel] [PATCH RFC v3 0/6] x86/idle: add halt poll support

[Quan Xu]

On 2017-11-16 05:31, Konrad Rzeszutek Wilk wrote:
> On Mon, Nov 13, 2017 at 06:05:59PM +0800, Quan Xu wrote:
>> From: Yang Zhang <yang.zhang.wz@gmail.com>
>>
>> Some latency-intensive workload have seen obviously performance
>> drop when running inside VM. The main reason is that the overhead
>> is amplified when running inside VM. The most cost I have seen is
>> inside idle path.
> Meaning an VMEXIT b/c it is an 'halt' operation ? And then going
> back in guest (VMRESUME) takes time. And hence your latency gets
> all whacked b/c of this?
    Konrad, I can't follow 'b/c' here.. sorry.

> So if I understand - you want to use your _full_ timeslice (of the guest)
> without ever (or as much as possible) to go in the hypervisor?
     as much as possible.

> Which means in effect you don't care about power-saving or CPUfreq
> savings, you just want to eat the full CPU for snack?
   actually, we  care about power-saving. The poll duration is 
self-tuning, otherwise it is almost as the same as
   'halt=poll'. Also we always sent out with CPU usage of benchmark 
netperf/ctxsw. We got much more
   performance with limited promotion of CPU usage.


>> This patch introduces a new mechanism to poll for a while before
>> entering idle state. If schedule is needed during poll, then we
>> don't need to goes through the heavy overhead path.
> Schedule of what? The guest or the host?
   rescheduled of guest scheduler..
   it is the guest.


Quan
Alibaba Cloud
>
>