Re: [PATCH 1/2] x86/idle: add halt poll for halt idle

[Yang Zhang]

On 2017/6/22 22:23, Thomas Gleixner wrote:
> On Thu, 22 Jun 2017, root wrote:
> > --- a/arch/x86/kernel/process.c
> > +++ b/arch/x86/kernel/process.c
> > @@ -39,6 +39,10 @@
> >  #include <asm/desc.h>
> >  #include <asm/prctl.h>
> >
> > +#ifdef CONFIG_HYPERVISOR_GUEST
> > +unsigned long poll_threshold_ns;
> > +#endif
> > +
> >  /*
> >   * per-CPU TSS segments. Threads are completely 'soft' on Linux,
> >   * no more per-task TSS's. The TSS size is kept cacheline-aligned
> > @@ -313,6 +317,23 @@ static inline void play_dead(void)
> >  }
> >  #endif
> >
> > +#ifdef CONFIG_HYPERVISOR_GUEST
> > +void arch_cpu_idle_poll(void)
> > +{
> > +	ktime_t start, cur, stop;
> > +
> > +	if (poll_threshold_ns) {
> > +		start = cur = ktime_get();
> > +		stop = ktime_add_ns(ktime_get(), poll_threshold_ns);
> > +		do {
> > +			if (need_resched())
> > +				break;
> > +			cur = ktime_get();
> > +		} while (ktime_before(cur, stop));
> > +	}
> > +}
> > +#endif
>
> Aside of the whole approach being debatable, what's the reason to make this
> depend on CONFIG_HYPERVISOR_GUEST and to move it into x86. If that
> mechanism is worthwhile then it should go into the generic code and not
> into x86. There is absolutely nothing x86 specific in that patch.
>
> Also the CONFIG_HYPERVISOR_GUEST dependency is silly. Distro kernels ship
> with CONFIG_HYPERVISOR_GUEST=y so this also gets into affect on bare metal.

You are right. As Paolo suggested, i will integrate it with 
paravalization code.

> Thanks,
>
> 	tglx


--
Yang
Alibaba Cloud Computing