On Wed, Jan 01, 2020 at 07:53:45PM -0500, Arvind Sankar wrote:
On Wed, Jan 01, 2020 at 09:51:02PM +0100, Arnd Bergmann wrote:
On Wed, Jan 1, 2020 at 6:57 PM Paul Burton paulburton@kernel.org wrote:
diff --git a/arch/mips/include/asm/thread_info.h b/arch/mips/include/asm/thread_info.h index 4993db40482c..aceefc3f9a1a 100644 --- a/arch/mips/include/asm/thread_info.h +++ b/arch/mips/include/asm/thread_info.h @@ -50,10 +50,10 @@ struct thread_info { }
/* How to get the thread information struct from C. */ -register struct thread_info *__current_thread_info __asm__("$28");
static inline struct thread_info *current_thread_info(void) {
register struct thread_info *__current_thread_info __asm__("$28");return __current_thread_info;}
This looks like a nice fix, but are you sure it doesn't allow the compiler to reuse $28 for another purpose in the kernel under register pressure, which would break current_thread_info()?
I see in the MIPS ABI document that $28 is preserved across function calls, but I don't see any indication that a function is not allowed to modify it and later restore the original content.
ArndThe compiler can already do that even with a global definition.
The doc since gcc 9 [1] says:
"Accesses to the variable may be optimized as usual and the register remains available for allocation and use in any computations, provided that observable values of the variable are not affected."
and
"Furthermore, since the register is not reserved exclusively for the variable, accessing it from handlers of asynchronous signals may observe unrelated temporary values residing in the register."
I'm not sure if this was a change in gcc 9 or simply the doc was wrong earlier.
Should there be a -ffixed-28 cflag for MIPS? alpha and hexagon seem to have that and they also keep current_thread_info in a register.
Also, commit fe92da0f355e9 ("MIPS: Changed current_thread_info() to an equivalent supported by both clang and GCC") moved this from local to global because local apparently didn't work on clang?
[1] https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Global-Register-Variables.html
Yeah this patch appears to break booting malta_defconfig in QEMU when built with clang; additionally, there are a TON of warnings about this variable being uninitialized:
../arch/mips/include/asm/thread_info.h:57:9: warning: variable '__current_thread_info' is uninitialized when used here [-Wuninitialized] return __current_thread_info; ^~~~~~~~~~~~~~~~~~~~~ ../arch/mips/include/asm/thread_info.h:55:52: note: initialize the variable '__current_thread_info' to silence this warning register struct thread_info *__current_thread_info __asm__("$28"); ^ = NULL 1 warning generated.
Seems like this is expected according to that previous commit? I noticed there is another instance in arch/mips but it doesn't appear to affect everything.
https://github.com/ClangBuiltLinux/linux/issues/606
Cheers, Nathan