Re: [PATCH] Option to disable AMD C1E (allows dynticks to work)

Eduard-Gabriel Munteanu
2007-12-14T09:52:18Z

On Thu, 13 Dec 2007 23:58:50 +0100
Johannes Weiner <hannes-kernel@saeurebad.de> wrote:
> I would find this way more readable:
>
> if (lo & ENABLE_C1E_MASK) {
> #ifdef CONFIG_X86_AMD_C1E_WORKAROUND
> if (disable_amd_c1e) {
> ...
> #else
> return 1;
> #endif
> }

I wanted to avoid using too many tabs, as that would require the lines
inside the if block to be split too many times. I'll resubmit the patch
if you think it's necessary.

> Why does it require to be enabled by compile-time AND run-time? Is
> that something you might want to decide on every boot? Could we make
> it settable at boot-time xor at compile-time?

Making it settable only at boot-time means compiling in unnecessary
code. Also, making it settable only at compile-time would be bad for
distros that distribute binary kernels, as some users may experience
worse power savings with dynticks than with C1E (especially when C2, C3
and higher C-states are not available, due to brain-dead firmware).
This allows them to opt for leaving C1E enabled.

It is not required to enable it at both compile-time and boot-time. You
enable it at compile-time and then you _can disable_ it at boot-time.