| Message ID | 51F0DAF1.9060702@asianux.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Benjamin Herrenschmidt |
| Headers | show |
On Thu, 2013-07-25 at 15:59 +0800, Chen Gang wrote: > > For my opinion: one fix may like below (assume have removed max_cpus) > which is more reasonable for code readers. So instead of just failing to bring the secondary CPUs, but potentially still having a working system, you crash during boot.... potentially before a console is even visible. And this is good how ? Ben.
On 07/25/2013 04:06 PM, Benjamin Herrenschmidt wrote: > On Thu, 2013-07-25 at 15:59 +0800, Chen Gang wrote: >> >> For my opinion: one fix may like below (assume have removed max_cpus) >> which is more reasonable for code readers. > > So instead of just failing to bring the secondary CPUs, but potentially > still having a working system, you crash during boot.... potentially > before a console is even visible. And this is good how ? > Hmm... how about the above DBG("...") within this function ? One implementation of BUG_ON() is use printk() and coredump, if it is a critical failure, I suggest to use it (if console is really invisible, I guess still can generate the coredump). Hmm... But do you mean it really can be failed, but it is not a critical failure ? if so we need print the related warning message instead of. Thanks.
On Thu, 2013-07-25 at 16:22 +0800, Chen Gang wrote: > On 07/25/2013 04:06 PM, Benjamin Herrenschmidt wrote: > > On Thu, 2013-07-25 at 15:59 +0800, Chen Gang wrote: > >> > >> For my opinion: one fix may like below (assume have removed max_cpus) > >> which is more reasonable for code readers. > > > > So instead of just failing to bring the secondary CPUs, but potentially > > still having a working system, you crash during boot.... potentially > > before a console is even visible. And this is good how ? > > > > Hmm... how about the above DBG("...") within this function ? > > One implementation of BUG_ON() is use printk() and coredump, if it is a > critical failure, I suggest to use it (if console is really invisible, I > guess still can generate the coredump). Whatever ... looks like you don't feel like listening so I'm not going to waste my breath anymore, nor will I accept your patches. Ben.
On 07/25/2013 04:28 PM, Benjamin Herrenschmidt wrote: > On Thu, 2013-07-25 at 16:22 +0800, Chen Gang wrote: >> > On 07/25/2013 04:06 PM, Benjamin Herrenschmidt wrote: >>> > > On Thu, 2013-07-25 at 15:59 +0800, Chen Gang wrote: >>>> > >> >>>> > >> For my opinion: one fix may like below (assume have removed max_cpus) >>>> > >> which is more reasonable for code readers. >>> > > >>> > > So instead of just failing to bring the secondary CPUs, but potentially >>> > > still having a working system, you crash during boot.... potentially >>> > > before a console is even visible. And this is good how ? >>> > > >> > >> > Hmm... how about the above DBG("...") within this function ? >> > >> > One implementation of BUG_ON() is use printk() and coredump, if it is a >> > critical failure, I suggest to use it (if console is really invisible, I >> > guess still can generate the coredump). > Whatever ... looks like you don't feel like listening so I'm not going > to waste my breath anymore, nor will I accept your patches. I can understand, But 'patch' or 'patches' ? ;-) Thanks.
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)