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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.20.1612290955191.5007@nanos>
Date:   Thu, 29 Dec 2016 17:42:48 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Frederic Weisbecker <fweisbec@...il.com>
cc:     LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        James Hartsock <hartsjc@...hat.com>,
        Ingo Molnar <mingo@...nel.org>, Rik van Riel <riel@...hat.com>
Subject: Re: [PATCH] nohz: Fix collision between tick and other hrtimers

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.

Thanks,

	tglx