From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932779AbdJQGKO (ORCPT ); Tue, 17 Oct 2017 02:10:14 -0400 Received: from mga03.intel.com ([134.134.136.65]:44095 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932729AbdJQGKM (ORCPT ); Tue, 17 Oct 2017 02:10:12 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,389,1503385200"; d="scan'208";a="163449967" Subject: Re: [RFC PATCH v2 5/8] timers: keep sleep length updated as needed To: "Rafael J. Wysocki" Cc: Aubrey Li , tglx@linutronix.de, peterz@infradead.org, len.brown@intel.com, ak@linux.intel.com, tim.c.chen@linux.intel.com, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org References: <1506756034-6340-1-git-send-email-aubrey.li@intel.com> <1554921.dz8jk4n8cL@aspire.rjw.lan> <33404efd-a8e0-eb0b-4df8-9e7aa5a66dde@linux.intel.com> <35931824.svOhBhJZiB@aspire.rjw.lan> From: "Li, Aubrey" Message-ID: <2e464ec4-b573-434a-e3df-baa00d1f7284@linux.intel.com> Date: Tue, 17 Oct 2017 14:10:08 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <35931824.svOhBhJZiB@aspire.rjw.lan> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017/10/17 7:58, Rafael J. Wysocki wrote: > On Monday, October 16, 2017 8:46:41 AM CEST Li, Aubrey wrote: >> On 2017/10/14 8:56, Rafael J. Wysocki wrote: >>> On Saturday, September 30, 2017 9:20:31 AM CEST Aubrey Li wrote: >>>> sleep length indicates how long we'll be idle. Currently, it's updated >>>> only when tick nohz enters. These patch series make a new requirement >>>> with tick, so we should keep sleep length updated as needed >>> >>> So what exactly would be the problem with leaving things as they are? >> >> Previously ts->sleep_length is only updated when tick is stopped. >> >> As follows, in >> >> __tick_nohz_idle_enter() >> { >> if (can_stop_idle_tick() /* return true */) { >> tick_nohz_stop_sched_tick() >> | >> |-----> update sleep_length >> } >> } > > Which is logical, because the tick will get in the way if we don't stop it, > won't it? > The scenario here is, tick(4ms) is too long for a short idle(e.g. 4us). And there could be hundreds of short idle within one tick interval. >> >> Now ts->sleep_length is required out of tick_nohz_idle_enter(), so we want >> to update sleep_length every time we read it >> >> If we leave it unchanged, the prediction could read a sleep_length long time >> ago if the system keep ticking. > > Well, but does it make sense to estimate the sleep length without stopping > the tick? For example, for the first short idle, we just turned tick off last time, so we may get the sleep length = 3900us. Then we keep tick on, and the 100th short idle comes, then the original sleep length is still 3900us(not updated), but actually it should be e.g. 100us. > >>>> --- >>>> kernel/time/tick-sched.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c >>>> index d663fab..94fb9b8 100644 >>>> --- a/kernel/time/tick-sched.c >>>> +++ b/kernel/time/tick-sched.c >>>> @@ -1008,8 +1008,11 @@ void tick_nohz_irq_exit(void) >>>> */ >>>> ktime_t tick_nohz_get_sleep_length(void) >>>> { >>>> + struct clock_event_device *dev = __this_cpu_read(tick_cpu_device.evtdev); >>>> struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched); >>>> >>>> + ts->sleep_length = ktime_sub(dev->next_event, ktime_get()); >>>> + >>>> return ts->sleep_length; >>>> } >>>> >>>> >>> >>> I probably wouldn't do it this way ... >>> >>> >> >> May I know the detailed thoughts? > > That depends on the answer above. :-) Does the above explanation address the concern? Thanks, -Aubrey