Re: [RFC PATCH V4 4/5] cpuidle: driver for xen

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Mar 22, 2011 at 06:03:28PM +0530, Trinabh Gupta wrote:
> This patch implements a default cpuidle driver for xen.
> Earlier pm_idle was flipped to default_idle. Maybe there
> is a better way to ensure default_idle is called
> without using this cpuidle driver.

Please also CC the Xen devel mailing list (I did this for you)

I couldn't find it in the description, but I was wondering
what is that you are trying to fix? What is the problem description?

Two, you mention in your description that it was tested on x86 systems. did
you test this with Xen? If so, what version.

> 
> Signed-off-by: Trinabh Gupta <trinabh@linux.vnet.ibm.com>
> ---
> 
>  arch/x86/xen/setup.c |   42 +++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 41 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index a8a66a5..4fce4cd 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -9,6 +9,8 @@
>  #include <linux/mm.h>
>  #include <linux/pm.h>
>  #include <linux/memblock.h>
> +#include <linux/cpuidle.h>
> +#include <linux/slab.h>
>  
>  #include <asm/elf.h>
>  #include <asm/vdso.h>
> @@ -321,6 +323,44 @@ void __cpuinit xen_enable_syscall(void)
>  #endif /* CONFIG_X86_64 */
>  }
>  
> +static struct cpuidle_driver xen_idle_driver = {
> +	.name = "xen_idle",
> +	.owner = THIS_MODULE,
> +	.priority = 1,
> +};
> +
> +extern struct cpuidle_state state_default_state;
> +
> +static int setup_cpuidle(int cpu)
> +{
> +	struct cpuidle_device *dev = kzalloc(sizeof(struct cpuidle_device),
> +					GFP_KERNEL);

No checking to see if dev == NULL?
> +	int count = CPUIDLE_DRIVER_STATE_START;
> +	dev->cpu = cpu;
> +	dev->drv = &xen_idle_driver;
> +
> +	dev->states[count] = state_default_idle;
> +	count++;
> +
> +	dev->state_count = count;
> +
> +	if (cpuidle_register_device(dev))
> +		return -EIO; 
No cleanup of the 'dev' so that we don't leak memory?

> +	return 0;
> +}
> +
> +static int xen_idle_init(void)
> +{
> +	int retval, i;
> +	retval = cpuidle_register_driver(&xen_idle_driver);
> +
> +	for_each_online_cpu(i) {
> +		setup_cpuidle(i);
> +	}
> +
> +	return 0;
> +}
> +
>  void __init xen_arch_setup(void)
>  {
>  	xen_panic_handler_init();
> @@ -354,7 +394,7 @@ void __init xen_arch_setup(void)
>  #ifdef CONFIG_X86_32
>  	boot_cpu_data.hlt_works_ok = 1;
>  #endif
> -	pm_idle = default_idle;
> +	xen_idle_init();
>  	boot_option_idle_override = IDLE_HALT;
>  
>  	fiddle_vdso();
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/