[PATCH] ARM: Add safe diagnostic to indicate when __cpu_architecture isn't set up

[nicolas.pitre@linaro.org (Nicolas Pitre)]

On Tue, 16 Aug 2011, Dave Martin wrote:

> On Tue, Aug 16, 2011 at 10:59:18AM -0400, Nicolas Pitre wrote:
> > On Tue, 16 Aug 2011, Tixy wrote:
> > 
> > > On Tue, 2011-08-16 at 15:19 +0100, Dave Martin wrote:
> > > [...]
> > > > This patch is useful for debugging, but I'm not convinced it should
> > > > be merged.
> > > [...] 
> > > >  static inline int __pure cpu_architecture(void)
> > > >  {
> > > > -	BUG_ON(__cpu_architecture == CPU_ARCH_UNKNOWN);
> > > > -	return __cpu_architecture;
> > > > +	if (unlikely(__cpu_architecture == CPU_ARCH_UNKNOWN)) {
> > > > +		extern int __pure __get_cpu_architecture(void);
> > > > +
> > > > +		WARN_ONCE(1, "__cpu_architecture not set yet!\n");
> > > > +		return __get_cpu_architecture();
> > > > +	} else
> > > > +		return __cpu_architecture;
> > > >  }
> > > 
> > > Seems to me that if we go down this route, cpu_architecture() may as
> > > well remain a non-inline function which just calculates the arch if it's
> > > not already set...
> > 
> > My thought as well.
> > 
> > > 	if (unlikely(__cpu_architecture == CPU_ARCH_UNKNOWN))
> > > 		__cpu_architecture = __get_cpu_architecture();
> > > 	return __cpu_architecture;
> > > 
> > > There seems to be too many ways to skin this cat :-)
> > 
> > Agreed.
> 
> Well, I guess the flipside is, do we need this check at all?
> 
> Because __cpu_architecture is set really early, and because if
> it's referenced too early, the kernel will either not boot or
> we'll hit BUG(), it's hard to imaging such a failure going unnoticed.
> 
> So this check could simply be viewed as overkill, though it does
> feel safer to check.
> 
> Any thoughts?

I personally think this is overkill.


Nicolas