On Sat, Sep 1, 2018 at 9:33 AM, Linus Torvalds <torva...@linux-foundation.org> wrote: > On Fri, Aug 31, 2018 at 3:21 PM Andy Lutomirski <l...@kernel.org> wrote: >> >> #ifdef CONFIG_X86_64 >> # define cpu_current_top_of_stack (cpu_tss_rw + TSS_sp1) >> +# define rsp_scratch (cpu_tss_rw + TSS_sp2) >> #endif > > Ugh. The above gets used by *assembler* code. I was really confused by how > this: > > >> --- a/arch/x86/kernel/process_64.c >> +++ b/arch/x86/kernel/process_64.c >> @@ -59,8 +59,6 @@ >> #include <asm/unistd_32_ia32.h> >> #endif >> >> -__visible DEFINE_PER_CPU(unsigned long, rsp_scratch); >> - > > could continue to work despite the accesses to "rsp_scratch" still > remaining in the asm files. > > Can yu humor me, and just not do something quite that subtle. I must > have missed this the first time around. > > Please get rid of the define, and just make the asm code spell out > what it actually does.
Done for v2. > > We already do that for TSS_sp0 for the normal case: > > movq PER_CPU_VAR(cpu_tss_rw + TSS_sp0), %rsp > > so I think this should just change > > - movq %rsp, PER_CPU_VAR(rsp_scratch) > + movq %rsp, PER_CPU_VAR(cpu_tss_rw + TSS_sp2) > > instead of having that subtle rsp_scratch thing. > > And honestly, I think we should strive to do the same thing with > cpu_current_top_of_stack. There at least the #define currently makes > sense (because on 32-bit, it's actually a percpu variable, on 64-bit > it's that sp1 field). > > But wouldn't it be nice to just unify 32-bit and 64-bit in this > respect, and get rid of that subtle difference? > Yes. But ugh, the way that thing has worked has changed so many times on 32-bit and 64-bit that I've lost track a little bit. I'll put it on my long list of things to clean up.
