linux-kernel - Re: [patch 18/39] tick: sched: Force tick interrupt and get rid of softirq magic

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.11.1504221627190.13914@nanos>
Date:	Wed, 22 Apr 2015 16:32:11 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Frederic Weisbecker <fweisbec@...il.com>
cc:	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Preeti U Murthy <preeti@...ux.vnet.ibm.com>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Marcelo Tosatti <mtosatt@...hat.com>,
	John Stultz <john.stultz@...aro.org>,
	Marcelo Tosatti <mtosatti@...hat.com>
Subject: Re: [patch 18/39] tick: sched: Force tick interrupt and get rid of
 softirq magic

On Wed, 22 Apr 2015, Frederic Weisbecker wrote:

> On Tue, Apr 14, 2015 at 09:08:54PM -0000, Thomas Gleixner wrote:
> > We already got rid of the hrtimer reprogramming loops and hoops as
> > hrtimer now enforces an interrupt if the enqueued time is in the past.
> > 
> > Do the same for the nohz non highres mode. That gets rid of the need
> > to raise the softirq which only serves the purpose of getting the
> > machine out of the inner idle loop.
> > 
> > Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> > Cc: John Stultz <john.stultz@...aro.org>
> > Cc: Preeti U Murthy <preeti@...ux.vnet.ibm.com>
> > Cc: Marcelo Tosatti <mtosatti@...hat.com>
> > Cc: Frederic Weisbecker <fweisbec@...il.com>
> > 
> > ---
> >  kernel/time/tick-sched.c |   83 ++++++++++++++++-------------------------------
> >  1 file changed, 29 insertions(+), 54 deletions(-)
> > 
> > Index: tip/kernel/time/tick-sched.c
> > ===================================================================
> > --- tip.orig/kernel/time/tick-sched.c
> > +++ tip/kernel/time/tick-sched.c
> > @@ -565,6 +565,20 @@ u64 get_cpu_iowait_time_us(int cpu, u64
> >  }
> >  EXPORT_SYMBOL_GPL(get_cpu_iowait_time_us);
> >  
> > +static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
> > +{
> > +	hrtimer_cancel(&ts->sched_timer);
> > +	hrtimer_set_expires(&ts->sched_timer, ts->last_tick);
> > +
> > +	/* Forward the time to expire in the future */
> > +	hrtimer_forward(&ts->sched_timer, now, tick_period);
> > +
> > +	if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
> > +		hrtimer_start_expires(&ts->sched_timer, HRTIMER_MODE_ABS_PINNED);
> > +	else
> > +		tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
> > +}
> > +
> >  static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
> >  					 ktime_t now, int cpu)
> >  {
> > @@ -691,22 +705,18 @@ static ktime_t tick_nohz_stop_sched_tick
> >  			if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
> >  				hrtimer_cancel(&ts->sched_timer);
> >  			goto out;
> > -		}
> > +		 }
> >  
> > -		if (ts->nohz_mode == NOHZ_MODE_HIGHRES) {
> > -			hrtimer_start(&ts->sched_timer, expires,
> > -				      HRTIMER_MODE_ABS_PINNED);
> > -			goto out;
> > -		} else if (!tick_program_event(expires, 0))
> > -			goto out;
> > -		/*
> > -		 * We are past the event already. So we crossed a
> > -		 * jiffie boundary. Update jiffies and raise the
> > -		 * softirq.
> > -		 */
> > -		tick_do_update_jiffies64(ktime_get());
> > +		 if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
> > +			 hrtimer_start(&ts->sched_timer, expires,
> > +				       HRTIMER_MODE_ABS_PINNED);
> > +		 else
> > +			 tick_program_event(expires, 1);
> > +	} else {
> > +		/* Tick is stopped, but required now. Enforce it */
> > +		tick_nohz_restart(ts, now);
> >  	}
> > -	raise_softirq_irqoff(TIMER_SOFTIRQ);
> > +
> 
> But the reprogramming happens only under "if ((long)delta_jiffies >= 1)".
> Probably this condition should go away as well.

Errm.

	if (!ts->tick_stopped && delta_jiffies <= 1)
	     goto out;

So if the tick is NOT stopped and delta_jiffies <= 1 we let it tick
and do nothing.

        if (delta_jiffies >= 1)
	     Do the magic nohz stuff
	else
	     tick_nohz_restart()

We want the distinction here because if the tick IS stopped and the
next event is due we need to kick it into gear again. So the condition
needs to stay. It probably should be if (delta > 1), but that's a
different story.
 
> In the end, the possible side effect, at least on low-res, is that timers
> which are already expired will be handled on the next tick instead of now.
> But probably it doesn't matter much to have a one-tick delay.

We really don't care. That stuff has no guarantees aside of the
guarantee that it does not expire early :)

Thanks,

	tglx

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/