Re: [PATCH] powerpc/machdep: warn when machine_is() used too early

[Michael Ellerman <mpe@ellerman.id.au>]

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 11/02/2023 à 00:56, Nathan Lynch via B4 Submission Endpoint a écrit :
>> From: Nathan Lynch <nathanl@linux.ibm.com>
>> 
>> machine_is() can't provide correct results before probe_machine() has
>> run. Warn when it's used too early in boot.
>> 
>> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
>> ---
>> Prompted by my attempts to do some pseries-specific setup during
>> rtas_initialize() and being puzzled for a while that it wasn't
>> working.
>> ---
>>   arch/powerpc/include/asm/machdep.h | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
>> index 378b8d5836a7..8c0a799d18cd 100644
>> --- a/arch/powerpc/include/asm/machdep.h
>> +++ b/arch/powerpc/include/asm/machdep.h
>> @@ -220,11 +220,13 @@ extern struct machdep_calls *machine_id;
>>   	EXPORT_SYMBOL(mach_##name);				\
>>   	struct machdep_calls mach_##name __machine_desc =
>>   
>> -#define machine_is(name) \
>> -	({ \
>> -		extern struct machdep_calls mach_##name \
>> -			__attribute__((weak));		 \
>> -		machine_id == &mach_##name; \
>> +#define machine_is(name)                                            \
>> +	({                                                          \
>> +		extern struct machdep_calls mach_##name             \
>> +			__attribute__((weak));                      \
>> +		WARN(!machine_id,                                   \
>> +		     "machine_is() called before probe_machine()"); \
>
> Is a WARN() really necessary ? WARN() is less optimised than WARN_ON(), 
> especially on PPC64.
>
> This should never ever happen so a WARN_ON(!machine_id) should be 
> enough, the developper that hits it is able to go to the given file:line 
> and understand what happened.

Yeah I agree, WARN_ON() should be sufficient here, and should generate
slightly better code. We have > 100 uses of machine_is(), so keeping
each small is desirable.

cheers