Re: [KJ] [PATCH] apm: clean up module initalization

Hi Neil,

On Tue, 1 Aug 2006 12:25:48 -0400 Neil Horman <[email protected]> wrote:
>
> diff --git a/arch/i386/kernel/apm.c b/arch/i386/kernel/apm.c
> index 8591f2f..75d8bde 100644
> --- a/arch/i386/kernel/apm.c
> +++ b/arch/i386/kernel/apm.c
> @@ -2339,16 +2339,22 @@ #endif
>  	ret = kernel_thread(apm, NULL, CLONE_KERNEL | SIGCHLD);
>  	if (ret < 0) {
>  		printk(KERN_ERR "apm: disabled - Unable to start kernel thread.\n");
> -		return -ENOMEM;
> +		ret = -ENOMEM;

Doesn't kernel_thread return a good error code?

> +		goto out_proc;

Good catch.

>  	}
>  
>  	if (num_online_cpus() > 1 && !smp ) {
>  		printk(KERN_NOTICE
>  		   "apm: disabled - APM is not SMP safe (power off active).\n");
> -		return 0;
> +		ret = -EOPNOTSUPP;
> +		goto out_thread;

This is a success return.  i.e. we want the kernel thread and the proc
file.  So the original was correct.  We have checked above if we are
trying to run SMP without the smp boot option - this check is just to stop
any more initialisation.

>  	}
>  
> -	misc_register(&apm_device);
> +	if (misc_register(&apm_device)) {
> +		printk(KERN_ERR "apm: Unable to register misc device\n");
> +		ret = -ENOMEM;
> +		goto out_thread;
> +	}

We don't care if misc_register fails as the apm module can still do much
good work even if user mode cannot control it.  So the original was
correct.  Maybe a comment here would be good as this patch has been
submitted before.

>  
>  	if (HZ != 100)
>  		idle_period = (idle_period * HZ) / 100;
> @@ -2357,8 +2363,17 @@ #endif
>  		pm_idle  = apm_cpu_idle;
>  		set_pm_idle = 1;
>  	}
> +	
> +	return  0;
>  
> -	return 0;
> +out_thread:
> +	exit_kapmd = 1;
> +	while (kapmd_running)
> +		schedule();

So we don't need the above.

-- 
Cheers,
Stephen Rothwell                    [email protected]
http://www.canb.auug.org.au/~sfr/

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)

iD8DBQFEz/oLFdBgD/zoJvwRAuljAJ4y8xhX5AtXNDPTRGcMCJnBOx4QBACfTeuG
cfQ+iFkfyMZ2wOoP5OCIadA=
=bnuq
-----END PGP SIGNATURE-----