Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1548203

Re: [PATCH] nohz: Fix collision between tick and other hrtimers

From Frederic Weisbecker <fweisbec@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH] nohz: Fix collision between tick and other hrtimers
Date 2016-12-29 18:00 +0100
Message-ID <sTMdz-E9-11@gated-at.bofh.it> (permalink)
References <sRXd8-Kq-17@gated-at.bofh.it> <sTM3U-AQ-19@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Dec 29, 2016 at 05:42:48PM +0100, Thomas Gleixner wrote:
> On Sat, 24 Dec 2016, Frederic Weisbecker wrote:
> >  static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
> >  					 ktime_t now, int cpu)
> >  {
> > -	struct clock_event_device *dev = __this_cpu_read(tick_cpu_device.evtdev);
> >  	u64 basemono, next_tick, next_tmr, next_rcu, delta, expires;
> >  	unsigned long seq, basejiff;
> >  	ktime_t	tick;
> > @@ -767,7 +766,7 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
> >  	tick.tv64 = expires;
> >  
> >  	/* Skip reprogram of event if its not changed */
> > -	if (ts->tick_stopped && (expires == dev->next_event.tv64))
> > +	if (ts->tick_stopped && (expires == ts->next_tick.tv64))
> >  		goto out;
> 
> Good catch!
> 
> >  
> >  	/*
> > @@ -787,6 +786,8 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
> >  		trace_tick_stop(1, TICK_DEP_MASK_NONE);
> >  	}
> >  
> > +	ts->next_tick = tick;
> > +
> >  	/*
> >  	 * If the expiration time == KTIME_MAX, then we simply stop
> >  	 * the tick timer.
> > @@ -803,7 +804,7 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
> >  		tick_program_event(tick, 1);
> >  out:
> >  	/* Update the estimated sleep length */
> > -	ts->sleep_length = ktime_sub(dev->next_event, now);
> > +	ts->sleep_length = ktime_sub(ts->next_tick, now);
> 
> This is wrong. If the next event is earlier than the next estimated tick
> then tick_nohz_get_sleep_length() will return crap and the idle governor
> will go into a deeper C-state than sensible.

Ah I see, the governor wants to know about the next timer, whether it is the tick
or not, right? I'll fix that and improve the comment along.

Thanks.

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH] nohz: Fix collision between tick and other hrtimers Frederic Weisbecker <fweisbec@gmail.com> - 2016-12-24 17:20 +0100
  Re: [PATCH] nohz: Fix collision between tick and other hrtimers Rik van Riel <riel@redhat.com> - 2016-12-26 04:00 +0100
    Re: [PATCH] nohz: Fix collision between tick and other hrtimers Frederic Weisbecker <fweisbec@gmail.com> - 2016-12-26 17:50 +0100
  Re: [PATCH] nohz: Fix collision between tick and other hrtimers Wanpeng Li <kernellwp@gmail.com> - 2016-12-27 00:50 +0100
  Re: [PATCH] nohz: Fix collision between tick and other hrtimers Thomas Gleixner <tglx@linutronix.de> - 2016-12-29 17:50 +0100
    Re: [PATCH] nohz: Fix collision between tick and other hrtimers Frederic Weisbecker <fweisbec@gmail.com> - 2016-12-29 18:00 +0100

csiph-web