Re: [PATCH V2 1/2] sched: idle: Encapsulate the code to compile it out

From: Daniel Lezcano
Date: Fri May 02 2014 - 09:29:51 EST


On 05/02/2014 02:09 PM, Rafael J. Wysocki wrote:
On Friday, May 02, 2014 10:59:11 AM Daniel Lezcano wrote:
On 05/01/2014 12:56 AM, Rafael J. Wysocki wrote:
On Thursday, May 01, 2014 12:47:25 AM Rafael J. Wysocki wrote:
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another
function so when CONFIG_CPU_IDLE=n, the code will be compiled out.
Also that is benefitial for the clarity of the code as it removes
a nested indentation level.

Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>

Well, this conflicts with

https://patchwork.kernel.org/patch/4071541/

which you haven't commented on and I still want cpuidle_select() to be able to
return negative values because of

https://patchwork.kernel.org/patch/4089631/

(and I have one more patch on top of these two that requires this).

Moreover (along the lines of Nico said) after https://patchwork.kernel.org/patch/4071541/
we actually don't need the #ifdef CONFIG_CPU_IDLE in your patch, because cpuidle_select()
for CONFIG_CPU_IDLE unset is a static inline returning a negative number and the compiler
should optimize out the blocks that depend on it being non-negative.

Thanks for the head up.

Actually that was to solve a compilation issue with the next patch when
adding the cpuidle state in the struct rq.

When the option CPU_IDLE is not set, the code assinging the cpu idle
state in the rq is still there while in the struct rq the field is
compiled out with the ifdef macro. If I rely on the compiler
optimization, the compilation error will happen.

I see.

If you don't put the new idle_state field in struct_rq under the #ifdef,
you won't need to worry about the build problem.

Alternatively, you can define

#ifdef CONFIG_CPU_IDLE
static inline void rq_set_idle_state(struct rq *rq, struct cpuidle_state *state)
{
rq->idle_state = state;
}
#else
static inline void rq_set_idle_state(struct rq *rq, struct cpuidle_state *state) {}
#endif

and use rq_set_idle_state() to set that field.

Thanks, I will look at one or another solution.




--
<http://www.linaro.org/> Linaro.org â Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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