Re: add lowpower_idle sysctl

[Andrew Morton <akpm@osdl.org>]

"Kenneth Chen" <kenneth.w.chen@intel.com> wrote:
>
> On ia64, we need runtime control to manage CPU power state in the idle
> loop. 

Can you expand on this?  Does this mean that the admin can select different
idle-loop algorithms?  If so, what alternative algorithms exist?

On x86 I'd like to be able to turn idle=poll on when performing oprofile
run, so the numbers come out right.  Will this let me do that?


> Logically it means a sysctl entry in /proc/sys/kernel.

Yes, but the *meanings* of the different values of that sysctl need to be
defined, and documented.  If lowpower_idle=42 has a totally different
meaning on different architectures then that's unfortunate but
understandable.  But we should at least enumerate the different values and
try to get different architectures to honour `42' in the same way.

> +atomic_t halt_counter;

Needs to be initialised - atomic_t's may have spinlocks inside them or
anything else.

> +extern atomic_t halt_counter;

Needs to be in a header, not in .c

>  /* this is needed for the proc_dointvec_minmax for [fs_]overflow UID and GID */
>  static int maxolduid = 65535;
> @@ -615,6 +616,14 @@
>  		.mode		= 0444,
>  		.proc_handler	= &proc_dointvec,
>  	},
> +	{
> +		.ctl_name	= KERN_LOWPOWER_IDLE,
> +		.procname	= "lowpower_idle",
> +		.data		= &halt_counter,
> +		.maxlen		= sizeof (int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec,
> +	},
>  	{ .ctl_name = 0 }
>  };

You cannot treat an int* as an atomic_t*!

Why do we want to inc and dec a user-specified tunable anyway?  I think I
don't understand what you're trying to do with this patch.