linux-kernel - Re: [patch 2/4] nohz: Prevent erroneous tick stop invocations

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.1712271923230.2431@nanos>
Date:   Wed, 27 Dec 2017 19:24:51 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Frederic Weisbecker <frederic@...nel.org>
cc:     LKML <linux-kernel@...r.kernel.org>,
        Anna-Maria Gleixner <anna-maria@...utronix.de>,
        Sebastian Siewior <bigeasy@...utronix.de>,
        Paul McKenney <paulmck@...ux.vnet.ibm.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Frederic Weisbecker <fweisbec@...il.com>,
        Ingo Molnar <mingo@...nel.org>, stable@...r.kernel.org
Subject: Re: [patch 2/4] nohz: Prevent erroneous tick stop invocations

On Wed, 27 Dec 2017, Thomas Gleixner wrote:
> On Tue, 26 Dec 2017, Frederic Weisbecker wrote:
> 
> > On Fri, Dec 22, 2017 at 03:51:13PM +0100, Thomas Gleixner wrote:
> > > The conditions in irq_exit() to invoke tick_nohz_irq_exit() are:
> > > 
> > >   if ((idle_cpu(cpu) && !need_resched()) || tick_nohz_full_cpu(cpu))
> > > 
> > > This is too permissive in various aspects:
> > > 
> > >   1) If need_resched() is set, then the tick cannot be stopped whether
> > >      the CPU is idle or in nohz full mode.
> > 
> > That's not exactly true. In nohz full mode the tick is not restarted on the
> > switch from idle to a single task. And if an idle interrupt wakes up a
> > single task and enqueues a timer, we want that timer to be programmed even
> > though we have need_resched().
> 
> Hrmm, so the check for softirq_pending() should be sufficient, right?
> 
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -382,7 +382,8 @@ static inline void tick_irq_exit(void)
>  	int cpu = smp_processor_id();
>  
>  	/* Make sure that timer wheel updates are propagated */
> -	if ((idle_cpu(cpu) && !need_resched()) || tick_nohz_full_cpu(cpu)) {
> +	if (((idle_cpu(cpu) && !need_resched()) || tick_nohz_full_cpu(cpu)) &&
> +	if ((idle_cpu(cpu) || tick_nohz_full_cpu(cpu)) &&
> +	    !local_softirq_pending()) {
>  		if (!in_interrupt())
>  			tick_nohz_irq_exit();
>  	}

Bah, no. We need to move that into the nohz logic somehow to prevent that
repetitive expiry yesterday reprogramming. Lemme think about it some more.

Thanks,

	tglx