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

On Tue, 16 Aug 2011, Dave Martin wrote:

On Tue, Aug 16, 2011 at 10:59:18AM -0400, Nicolas Pitre wrote:

quoted

On Tue, 16 Aug 2011, Tixy wrote:

quoted

On Tue, 2011-08-16 at 15:19 +0100, Dave Martin wrote:
[...]

quoted

This patch is useful for debugging, but I'm not convinced it should
be merged.

[...] 

quoted

 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.

quoted

	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