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

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 17/07/2023 à 07:01, Michael Ellerman a écrit :
> 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.
> 

Ok that's fine for me.

I'll do that in a follow-up patch one day.

Christophe