Re: [PATCH v5] Revert "powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto"

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

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 12/07/2023 à 15:45, Michael Ellerman a écrit :
>> From: Christophe Leroy <christophe.leroy@csgroup.eu>
>> 
>> This partly reverts commit 1e688dd2a3d6759d416616ff07afc4bb836c4213.
>> 
>> That commit aimed at optimising the code around generation of
>> WARN_ON/BUG_ON but this leads to a lot of dead code erroneously
>> generated by GCC.
>> 
>> That dead code becomes a problem when we start using objtool validation
>> because objtool will abort validation with a warning as soon as it
>> detects unreachable code. This is because unreachable code might
>> be the indication that objtool doesn't properly decode object text.
>> 
>>       text	   data	    bss	    dec	    hex	filename
>>    9551585	3627834	 224376	13403795	 cc8693	vmlinux.before
>>    9535281	3628358	 224376	13388015	 cc48ef	vmlinux.after
>> 
>> Once this change is reverted, in a standard configuration (pmac32 +
>> function tracer) the text is reduced by 16k which is around 1.7%
>> 
>> We already had problem with it when starting to use objtool on powerpc
>> as a replacement for recordmcount, see commit 93e3f45a2631 ("powerpc:
>> Fix __WARN_FLAGS() for use with Objtool")
>> 
>> There is also a problem with at least GCC 12, on ppc64_defconfig +
>> CONFIG_CC_OPTIMIZE_FOR_SIZE=y + CONFIG_DEBUG_SECTION_MISMATCH=y :
>> 
>>      LD      .tmp_vmlinux.kallsyms1
>>    powerpc64-linux-ld: net/ipv4/tcp_input.o:(__ex_table+0xc4): undefined reference to `.L2136'
>>    make[2]: *** [scripts/Makefile.vmlinux:36: vmlinux] Error 1
>>    make[1]: *** [/home/chleroy/linux-powerpc/Makefile:1238: vmlinux] Error 2
>> 
>> Taking into account that other problems are encountered with that
>> 'asm goto' in WARN_ON(), including build failures, keeping that
>> change is not worth it allthough it is primarily a compiler bug.
>> 
>> Revert it for now.
>> 
>> mpe: Retain EMIT_WARN_ENTRY as a synonym for EMIT_BUG_ENTRY to reduce
>> churn, as there are now nearly as many uses of EMIT_WARN_ENTRY as
>> EMIT_BUG_ENTRY.
>
> In that case, should we keep __EMIT_BUG_ENTRY and also keep the check 
> that makes sure nobody uses EMIT_BUG_ENTRY with BUGFLAG_WARNING ?

I didn't think it was worth it, now that it's not a correctness issue.

I think the better option would be to have EMIT_WARN_ENTRY add
BUGFLAG_WARNING itself, rather than the caller having to pass it.

cheers