linux-kernel - Re: [PATCH v9 05/10] cpuidle: Return nohz hint from cpuidle

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]

Date:   Fri, 06 Apr 2018 09:24:42 +0200
From:   "Rafael J. Wysocki" <rjw@...ysocki.net>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     Linux PM <linux-pm@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Frederic Weisbecker <fweisbec@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Paul McKenney <paulmck@...ux.vnet.ibm.com>,
        Thomas Ilsche <thomas.ilsche@...dresden.de>,
        Doug Smythies <dsmythies@...us.net>,
        Rik van Riel <riel@...riel.com>,
        Aubrey Li <aubrey.li@...ux.intel.com>,
        Mike Galbraith <mgalbraith@...e.de>,
        LKML <linux-kernel@...r.kernel.org>,
        Len Brown <len.brown@...el.com>
Subject: Re: [PATCH v9 05/10] cpuidle: Return nohz hint from cpuidle_select()

On Friday, April 6, 2018 4:44:14 AM CEST Frederic Weisbecker wrote:
> On Wed, Apr 04, 2018 at 10:39:50AM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > Index: linux-pm/kernel/time/tick-sched.c
> > ===================================================================
> > --- linux-pm.orig/kernel/time/tick-sched.c
> > +++ linux-pm/kernel/time/tick-sched.c
> > @@ -991,6 +991,20 @@ void tick_nohz_irq_exit(void)
> >  }
> >  
> >  /**
> > + * tick_nohz_idle_got_tick - Check whether or not the tick handler has run
> > + */
> > +bool tick_nohz_idle_got_tick(void)
> > +{
> > +	struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);
> > +
> > +	if (ts->inidle > 1) {
> > +		ts->inidle = 1;
> > +		return true;
> > +	}
> > +	return false;
> > +}
> > +
> > +/**
> >   * tick_nohz_get_sleep_length - return the length of the current sleep
> >   *
> >   * Called from power state control code with interrupts disabled
> > @@ -1101,6 +1115,9 @@ static void tick_nohz_handler(struct clo
> >  	struct pt_regs *regs = get_irq_regs();
> >  	ktime_t now = ktime_get();
> >  
> > +	if (ts->inidle)
> > +		ts->inidle = 2;
> > +
> 
> You can move that to tick_sched_do_timer() to avoid code duplication.

Right.

> Also these constants are very opaque. And even with proper symbols it wouldn't look
> right to extend ts->inidle that way.

Well, this was a Peter's idea. :-)

> Perhaps you should add a field such as ts->got_idle_tick under the boolean fields
> after the below patch:

OK, but at this point I'd prefer to make such changes on top of the existing
set, because that's got quite some testing coverage already and honestly this
is cosmetics in my view (albeit important).

> --
> From c7b2ca5a4c512517ddfeb9f922d5999f82542ced Mon Sep 17 00:00:00 2001
> From: Frederic Weisbecker <frederic@...nel.org>
> Date: Fri, 6 Apr 2018 04:32:37 +0200
> Subject: [PATCH] nohz: Gather tick_sched booleans under a common flag field
> 
> This optimize the space and leave plenty of room for further flags.
> 
> Signed-off-by: Frederic Weisbecker <frederic@...nel.org>
> ---
>  kernel/time/tick-sched.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/time/tick-sched.h b/kernel/time/tick-sched.h
> index 954b43d..38f24dc 100644
> --- a/kernel/time/tick-sched.h
> +++ b/kernel/time/tick-sched.h
> @@ -45,14 +45,17 @@ struct tick_sched {
>  	struct hrtimer			sched_timer;
>  	unsigned long			check_clocks;
>  	enum tick_nohz_mode		nohz_mode;
> +
> +	unsigned int			inidle		: 1;
> +	unsigned int			tick_stopped	: 1;
> +	unsigned int			idle_active	: 1;
> +	unsigned int			do_timer_last	: 1;
> +
>  	ktime_t				last_tick;
>  	ktime_t				next_tick;
> -	int				inidle;
> -	int				tick_stopped;
>  	unsigned long			idle_jiffies;
>  	unsigned long			idle_calls;
>  	unsigned long			idle_sleeps;
> -	int				idle_active;
>  	ktime_t				idle_entrytime;
>  	ktime_t				idle_waketime;
>  	ktime_t				idle_exittime;
> @@ -62,7 +65,6 @@ struct tick_sched {
>  	unsigned long			last_jiffies;
>  	u64				next_timer;
>  	ktime_t				idle_expires;
> -	int				do_timer_last;
>  	atomic_t			tick_dep_mask;
>  };
>  
>