linux-kernel - Re: [v10 PATCH 2/9]: cpuidle: cleanup drivers/cpuidle/cpuidle.c

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, 4 Dec 2009 23:20:00 +0100
From:	Torsten Duwe <duwe@....de>
To:	arun@...ux.vnet.ibm.com
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Ingo Molnar <mingo@...e.hu>,
	Vaidyanathan Srinivasan <svaidy@...ux.vnet.ibm.com>,
	Dipankar Sarma <dipankar@...ibm.com>,
	Balbir Singh <balbir@...ibm.com>,
	Venkatesh Pallipadi <venkatesh.pallipadi@...el.com>,
	linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
	linux-arch@...r.kernel.org, linux-acpi@...r.kernel.org
Subject: Re: [v10 PATCH 2/9]: cpuidle: cleanup drivers/cpuidle/cpuidle.c

On Wednesday 02 December 2009, Arun R Bharadwaj wrote:
> * Arun R Bharadwaj <arun@...ux.vnet.ibm.com> [2009-12-02 15:24:27]:
>
> This patch cleans up drivers/cpuidle/cpuidle.c
> Earlier cpuidle assumed pm_idle as the default idle loop. Break that
> assumption and make it more generic.

Is there a problem with the old pm_idle? Couldn't it be integrated more 
transparently, instead of replacing it this intrusively?

> --- linux.trees.git.orig/include/linux/cpuidle.h
> +++ linux.trees.git/include/linux/cpuidle.h
> @@ -41,7 +41,7 @@ struct cpuidle_state {
>  	unsigned long long	usage;
>  	unsigned long long	time; /* in US */
>
> -	int (*enter)	(struct cpuidle_device *dev,
> +	void (*enter)	(struct cpuidle_device *dev,
>  			 struct cpuidle_state *state);
>  };

While it may be a good idea to move the residency calculation to one central 
place, at least in theory a cpuidle_state->enter() function could have a 
better method to determine its value.

Either way you're implicitly introducing an API change here, and you're at 
least missing two functions on ARM and SuperH, respectively. Could you 
separate this API change out, and not take it for granted in the other 
patches?

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