Le 31/01/2023 à 03:58, Benjamin Gray a écrit :

The commit introducing this function implemented it as a build bug on this
platform to make the compiler happy, as the only use in the code is guarded
behind a radix_enabled() conditional.

GCC recognises that cpu_has_feature(MMU_FTR_TYPE_RADIX) returns false on this
platform and eliminates the build bug as dead code. However, when
CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG is enabled, the assertion and possible
call to early_cpu_... prevents Clang from eliminating any code that's
conditional on the return value. So Clang triggers the build bug as reported
by the kernel test robot:

I still think it is not the correct fix. You are putting the problem 
under the carpet instead of fixing it. There are many other places where 
radix_enabled() or other mmu_has_feature() are used with the expectation 
that one leg will be eliminated at build time.

As written in previous thread, have you considered reworking 
mmu_has_feature() to move the CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG 
after the below block:

	if (MMU_FTRS_ALWAYS & feature)
		return true;

	if (!(MMU_FTRS_POSSIBLE & feature))
		return false;


And as this looks like a Clang bug or limitation, can you provide us 
with a link to the Clang issue you have opened for it ?


Looking into it in more details, I'm even more puzzled. As far as I can 
see, local_flush_tlb_page_psize() is used only at one place, that is 
function __do_patch_instruction_mm(). So if Clang fails to identify it 
as a dead leg, it is the full __do_patch_instruction_mm() which is kept 
for no reason.

On the other hand, I see that local_flush_tlb_page_psize() implemented 
for nohash/32, so yes we can also implement it for book3s/32. But then 
the commit log should explain things differently.

By the way, I also see that local_flush_tlb_page_psize() for book3s/64 
does just nothing at all for non Radix. Is that correct ?

Thanks
Christophe