On 07/25/2013 03:33 PM, Benjamin Herrenschmidt wrote:

On Thu, 2013-07-25 at 14:17 +0800, Chen Gang wrote:

quoted

quoted

Hmm... for an extern function (espeically have been implemented in
various modules), normally, we can assume it may fail in some cases
(although now, we don't know what cases can cause its failure).

If "we don't have a good way to handle the failure", "print the related
warning message" is an executable choice (or "BUG_ON()", if it is critical).

So, if the performance is not sensible, I still suggest to let extern
function have return value.

This is not a module function. We are not doing a uni course on how to
write C code here. Be real.

In our case, 'module' points to various sub directories of arch/powerpc
(maybe 'module' is not quite precise, it is easy misunderstand).

The real world is not conflict with "how to write C code".

For my opinion: one fix may like below (assume have removed max_cpus)
which is more reasonable for code readers.

-----------------------------diff begin------------------------------

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 7edbd5b..53155f4 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c

@@ -347,7 +347,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 	cpumask_set_cpu(boot_cpuid, cpu_core_mask(boot_cpuid));
 
 	if (smp_ops && smp_ops->probe)
-		smp_ops->probe();
+		BUG_ON(smp_ops->probe() < 0);
 }
 
 void smp_prepare_boot_cpu(void)

-----------------------------diff end--------------------------------

Thanks
-- 
Chen Gang