linux-kernel - Re: [PATCHv4 1/4] cpuidle: refactor out cpuidle_enter

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: <201205092303.36059.rjw@sisk.pl>
Date:	Wed, 9 May 2012 23:03:35 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Colin Cross <ccross@...roid.com>
Cc:	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-pm@...ts.linux-foundation.org, Kevin Hilman <khilman@...com>,
	Len Brown <len.brown@...el.com>,
	Trinabh Gupta <g.trinabh@...il.com>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Deepthi Dharwar <deepthi@...ux.vnet.ibm.com>,
	"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
	Kay Sievers <kay.sievers@...y.org>,
	Santosh Shilimkar <santosh.shilimkar@...com>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Amit Kucheria <amit.kucheria@...aro.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
	Arnd Bergmann <arnd.bergmann@...aro.org>,
	Russell King <linux@....linux.org.uk>
Subject: Re: [PATCHv4 1/4] cpuidle: refactor out cpuidle_enter_state

On Tuesday, May 08, 2012, Colin Cross wrote:
> Split the code to enter a state and update the stats into a helper
> function, cpuidle_enter_state, and export it.  This function will
> be called by the coupled state code to handle entering the safe
> state and the final coupled state.
> 
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@...com>
> Tested-by: Santosh Shilimkar <santosh.shilimkar@...com>
> Reviewed-by: Kevin Hilman <khilman@...com>
> Tested-by: Kevin Hilman <khilman@...com>
> Signed-off-by: Colin Cross <ccross@...roid.com>

I've said

Reviewed-by: Rafael J. Wysocki <rjw@...k.pl>

to this already.  Any chance to update the tags?

Rafael


> ---
>  drivers/cpuidle/cpuidle.c |   42 +++++++++++++++++++++++++++++-------------
>  drivers/cpuidle/cpuidle.h |    2 ++
>  2 files changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 2f0083a..3e3e3e4 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -103,6 +103,34 @@ int cpuidle_play_dead(void)
>  }
>  
>  /**
> + * cpuidle_enter_state - enter the state and update stats
> + * @dev: cpuidle device for this cpu
> + * @drv: cpuidle driver for this cpu
> + * @next_state: index into drv->states of the state to enter
> + */
> +int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
> +		int next_state)
> +{
> +	int entered_state;
> +
> +	entered_state = cpuidle_enter_ops(dev, drv, next_state);
> +
> +	if (entered_state >= 0) {
> +		/* Update cpuidle counters */
> +		/* This can be moved to within driver enter routine
> +		 * but that results in multiple copies of same code.
> +		 */
> +		dev->states_usage[entered_state].time +=
> +				(unsigned long long)dev->last_residency;
> +		dev->states_usage[entered_state].usage++;
> +	} else {
> +		dev->last_residency = 0;
> +	}
> +
> +	return entered_state;
> +}
> +
> +/**
>   * cpuidle_idle_call - the main idle loop
>   *
>   * NOTE: no locks or semaphores should be used here
> @@ -143,23 +171,11 @@ int cpuidle_idle_call(void)
>  	trace_power_start_rcuidle(POWER_CSTATE, next_state, dev->cpu);
>  	trace_cpu_idle_rcuidle(next_state, dev->cpu);
>  
> -	entered_state = cpuidle_enter_ops(dev, drv, next_state);
> +	entered_state = cpuidle_enter_state(dev, drv, next_state);
>  
>  	trace_power_end_rcuidle(dev->cpu);
>  	trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
>  
> -	if (entered_state >= 0) {
> -		/* Update cpuidle counters */
> -		/* This can be moved to within driver enter routine
> -		 * but that results in multiple copies of same code.
> -		 */
> -		dev->states_usage[entered_state].time +=
> -				(unsigned long long)dev->last_residency;
> -		dev->states_usage[entered_state].usage++;
> -	} else {
> -		dev->last_residency = 0;
> -	}
> -
>  	/* give the governor an opportunity to reflect on the outcome */
>  	if (cpuidle_curr_governor->reflect)
>  		cpuidle_curr_governor->reflect(dev, entered_state);
> diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
> index 7db1866..d8a3ccc 100644
> --- a/drivers/cpuidle/cpuidle.h
> +++ b/drivers/cpuidle/cpuidle.h
> @@ -14,6 +14,8 @@
>  extern struct mutex cpuidle_lock;
>  extern spinlock_t cpuidle_driver_lock;
>  extern int cpuidle_disabled(void);
> +extern int cpuidle_enter_state(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int next_state);
>  
>  /* idle loop */
>  extern void cpuidle_install_idle_handler(void);
> 

--
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/