linux-kernel - Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm

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, 28 Aug 2009 12:29:08 +0530
From:	Balbir Singh <balbir@...ux.vnet.ibm.com>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	arun@...ux.vnet.ibm.com,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Joel Schopp <jschopp@...tin.ibm.com>,
	Paul Mackerras <paulus@...ba.org>, Ingo Molnar <mingo@...e.hu>,
	Vaidyanathan Srinivasan <svaidy@...ux.vnet.ibm.com>,
	Dipankar Sarma <dipankar@...ibm.com>,
	Gautham R Shenoy <ego@...ibm.com>,
	"Pallipadi, Venkatesh" <venkatesh.pallipadi@...el.com>,
	linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent
	cpuidle_pm_idle in drivers/cpuidle/cpuidle.c

* Peter Zijlstra <a.p.zijlstra@...llo.nl> [2009-08-28 08:48:05]:

> On Fri, 2009-08-28 at 11:44 +0530, Arun R Bharadwaj wrote:
> > * Peter Zijlstra <a.p.zijlstra@...llo.nl> [2009-08-27 14:53:27]:
> > 
> > Hi Peter, Ben,
> > 
> > I've put the whole thing in a sort of a block diagram. Hope it
> > explains things more clearly.
> > 
> > 
> > 
> > 
> > 
> > 
> > 				----------------
> > 				|    CPUIDLE   |  (Select idle states like
> > 				|   GOVERNORS  |   C1, C1e, C6 etc in case
> > 				| (Menu/Ladder)|   x86 & nap, snooze in
> > 				|	       |   case of POWER - based on
> > 				----------------   latency & power req)
> > 					^
> > 					|
> > 					|
> > 					|
> > 					|
> > 					|
> >   ----------			-----------------	         -------------
> >   |	   |			|		|	         |  PSERIES  |
> >   |  ACPI  |------------------>	|    CPUIDLE	| <--------------|   IDLE    |
> >   |	   |			|		|	         |           |
> >   ----------			-----------------	         -------------
> > 
> > Main idle routine- pm_idle()			         Main idle routine-
> > 							     ppc_md.power_save()
> > 
> > pm_idle = cpuidle_pm_idle;			         ppc_md.power_save =
> > (start using cpuidle's idle				      cpuidle_pm_idle();
> >  loop, which internally calls
> >  governor to select the right
> >  state to go into).
> > 
> > 
> > Relavent code snippet from drivers/cpuidle/cpuidle.c
> > -------------------------------------
> > 
> > static void cpuidle_idle_call(void)
> > {
> > 	............
> > 	............
> > 
> > 	/* Call the menu_select() to select the idle state to enter. */
> > 	next_state = cpuidle_curr_governor->select(dev);
> > 
> > 	............
> > 	............
> > 
> > 	/*
> > 	 * Enter the idle state previously selected. target_state->enter
> > 	 * would call pseries_cpuidle_loop() which selects nap/snooze
> > 	 * /
> > 	dev->last_residency = target_state->enter(dev, target_state);
> > }
> > 
> > void cpuidle_install_idle_handler(void)
> > {
> > 	.........
> > 	.........
> > 	cpuidle_pm_idle = cpuidle_idle_call;
> > }
> 
> All I'm seeing here is a frigging mess.
> 
> How on earths can something called: cpuidle_install_idle_handler() have
> a void argument, _WHAT_ handler is it going to install?
>

Peter, I think that is a typo, we need a function pointer like your
snippet of code showed
 
> So somehow you added to the ACPI mess by now having 3 wild function
> pointers, that's _NOT_ progress.
>

The goal, IIUC is to integrate three modules

1. CPUIdle - for idle CPU management
2. Architecture specific code for idle detection
3. CPUIdle governor

Again, if IIUC

The architecture specific idle code would call an idle loop that would
call into the CPUIdle framework, which inturn would depend on the
governor selected.

Passing void* is a mess, we need function pointer registration
and a framework so that we can query what is registered.


 

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