Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 1 Aug 2018 09:10:56 +0200 (CEST)
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Xu YiPing <xuyiping@hisilicon.com>
cc:     John Stultz <john.stultz@linaro.org>, sboyd@kernel.org,
        LKML <linux-kernel@vger.kernel.org>, anna-maria@linutronix.de
Subject: Re: [PATCH] timers: fix offset calculation when the expires align
 with LVL_GRAN
In-Reply-To: <86709eae-fa35-e36a-cafd-2c917d188276@hisilicon.com>
Message-ID: <alpine.DEB.2.21.1808010855500.1713@nanos.tec.linutronix.de>
References: <1532669610-103892-1-git-send-email-xuyiping@hisilicon.com> <alpine.DEB.2.21.1807301234310.2518@nanos.tec.linutronix.de> <c68d9944-1fcc-dcd5-1e3d-07f979323b8c@hisilicon.com> <alpine.DEB.2.21.1807311616480.1610@nanos.tec.linutronix.de>
 <alpine.DEB.2.21.1807311633430.1610@nanos.tec.linutronix.de> <86709eae-fa35-e36a-cafd-2c917d188276@hisilicon.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 1 Aug 2018, Xu YiPing wrote:
> On 2018/7/31 22:34, Thomas Gleixner wrote:
> > 
> > 
> > On Tue, 31 Jul 2018, Thomas Gleixner wrote:
> > 
> >> On Tue, 31 Jul 2018, Xu YiPing wrote:
> >>> On 2018/7/30 19:03, Thomas Gleixner wrote:
> >>>>
> >>>> __internal_add_timer(base, timer)
> >>>> {  
> >>>>    idx = calc_wheel_index(1, 1)
> >>>>        {
> >>>>            delta = 1 - 1;	<-   0
> >>>>
> >>>>            if (0 < LVL_START(1))
> >>>>                idx = calc_index(1, 0)
> >>>>                     {
> >>>>                         expires = (1 + LVL_GRAN(0) - 1) >> LVL_SHIFT(0);
> >>>> 			----> expires = 0
> >>> 			
> >>> 			LVL_GRAN(0) = 1 and LVL_SHIFT(0) = 0
> >>> 			
> >>> 			after the calculation, expires = 1 ?
> >>
> >> Indeed. You're right. Math is hard... So the index would be 1 and still not
> >> fulfil the below:
> > 
> > Hmm, crap. Let me think about it some more. 34C is way too hot to think.
> > 
> 
> 	when the expire is aligned with LVL_GRAN(x), it could be triggered at expire,
> no need to wait another LVL_GRAN(x).
> 
> 	just a little improvement, useful when expire is small.

The expire early problem still persists:

schedule_timeout(x)
   expires = jiffies + x;
   mod_timer(timer, expires);

This will return _BEFORE_ X jiffies have elapsed in 99% of the cases, so it's
not a little improvement, it's a big regression.

Again for schedule_timeout(1):

   |-------------------|-------------------|--------------------|
  tick               tick                tick			
 jiffies           jiffies + 1        jiffies + 2

   |                   |
   |  Any timer armed  |                   ^
   |  here must be     |                   |
   |  queued here -------------------------|
                       |                   |
                       |  Any timer armed  |                   ^
                       |  here must be     |                   |
                       |  queued here -------------------------|

The only case where your change would be correct is if _all_ timers are
armed exactly at the jiffies boundary. Can you guarantee that? Not at all.

And even hrtiemrs suffer from this issue when high resolution mode is
disabled because then they are expired on the tick boundary as well and if
the expiry time is past the current time at the tick boundary then the
timer will be expired one tick later.

It's a fundamental property of timers which are driven by a periodic tick
and you can argue in circles it wont change.

Thanks,

	tglx