Subject: Re: [PATCH 2/2] x86/idle: use dynamic halt poll
To: Thomas Gleixner <tglx@linutronix.de>
References: <1498130534-26568-1-git-send-email-root@ip-172-31-39-62.us-west-2.compute.internal>
 <1498130534-26568-3-git-send-email-root@ip-172-31-39-62.us-west-2.compute.internal>
 <alpine.DEB.2.20.1706221623230.1885@nanos>
Cc: mingo@redhat.com, hpa@zytor.com, pbonzini@redhat.com, x86@kernel.org,
        corbet@lwn.net, tony.luck@intel.com, bp@alien8.de,
        peterz@infradead.org, mchehab@kernel.org, akpm@linux-foundation.org,
        krzk@kernel.org, jpoimboe@redhat.com, luto@kernel.org,
        borntraeger@de.ibm.com, thgarnie@google.com, rgerst@gmail.com,
        minipli@googlemail.com, douly.fnst@cn.fujitsu.com, nicstange@gmail.com,
        fweisbec@gmail.com, dvlasenk@redhat.com, bristot@redhat.com,
        yamada.masahiro@socionext.com, mika.westerberg@linux.intel.com,
        yu.c.chen@intel.com, aaron.lu@intel.com, rostedt@goodmis.org,
        me@kylehuey.com, len.brown@intel.com, prarit@redhat.com,
        hidehiro.kawai.ez@hitachi.com, fengtiantian@huawei.com,
        pmladek@suse.com, jeyu@redhat.com, Larry.Finger@lwfinger.net,
        zijun_hu@htc.com, luisbg@osg.samsung.com, johannes.berg@intel.com,
        niklas.soderlund+renesas@ragnatech.se, zlpnobody@gmail.com,
        adobriyan@gmail.com, fgao@48lvckh6395k16k5.yundunddos.com,
        ebiederm@xmission.com, subashab@codeaurora.org, arnd@arndb.de,
        matt@codeblueprint.co.uk, mgorman@techsingularity.net,
        linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-edac@vger.kernel.org, kvm@vger.kernel.org
From: Yang Zhang <yang.zhang.wz@gmail.com>
Message-ID: <e6a78334-c9c1-0b0b-9d62-f6ade04cf93f@gmail.com>
Date: Fri, 23 Jun 2017 12:04:40 +0800
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.20.1706221623230.1885@nanos>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1698
Lines: 74

On 2017/6/22 22:32, Thomas Gleixner wrote:
> On Thu, 22 Jun 2017, root wrote:
>> @@ -962,6 +962,7 @@ __visible void __irq_entry smp_apic_timer_interrupt(struct pt_regs *regs)
>>  	 * interrupt lock, which is the WrongThing (tm) to do.
>>  	 */
>>  	entering_ack_irq();
>> +	check_poll();
>
> No way, that we sprinkle this function into every interrupt hotpath. There
> are enough genuine ways to do that w/o touching a gazillion of files.

I will find a more correct place to call this function.

>
>>  #ifdef CONFIG_HYPERVISOR_GUEST
>> +static unsigned int grow_poll_ns(unsigned int old, unsigned int grow,
>> +				      unsigned int max)
>> +{
>> +	unsigned int val;
>> +
>> +	/* 10us as base poll duration */
>> +	if (old == 0 && grow)
>> +		return 10000;
>> +
>> +	val = old * grow;
>> +	if (val > max)
>> +		val = max;
>> +
>> +	return val;
>> +}
>> +
>> +static unsigned int shrink_poll_ns(unsigned int old, unsigned int shrink)
>> +{
>> +	if (shrink == 0)
>> +		return 0;
>> +
>> +	return old / shrink;
>> +}
>> +
>> +void check_poll(void)
>> +{
>> +	unsigned int val, poll_duration;
>> +	unsigned long begin_ns, now_ns;
>> +
>> +	if (!poll_threshold_ns)
>> +		return;
>
> If at all then this needs to be a static key based decision.

Sure, will do it.

>
>> +
>> +	begin_ns = this_cpu_read(poll_begin_ns);
>> +	/* Not from halt state */
>> +	if (!begin_ns)
>> +		return;
>
> If you integrate this stuff into the proper place, then the whole mess goes
> away. We really do not need another facility to track idle state. We have
> enough already, really.

Agree, I will check current code to find a more proper way to do the check.

>
> Thanks,
>
> 	tglx
>


-- 
Yang
Alibaba Cloud Computing