Messages in this thread | | | Date | Fri, 24 Apr 2015 17:29:42 +0200 | From | Denys Vlasenko <> | Subject | Re: [PATCH 1/2] x86: stop using PER_CPU_VAR(kernel_stack) |
| |
On 03/23/2015 07:27 PM, Andy Lutomirski wrote: > On Mon, Mar 23, 2015 at 10:51 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote: >> On 03/23/2015 06:45 PM, Steven Rostedt wrote: >>> On Mon, 23 Mar 2015 18:38:09 +0100 >>> Denys Vlasenko <dvlasenk@redhat.com> wrote: >>> >>>>> Can we do a: >>>>> >>>>> #define cpu_current_top_of_stack (cpu_tss + TSS_sp0) >>>> >>>> We already do something similar: >>>> >>>> static inline unsigned long current_top_of_stack(void) >>>> { >>>> #ifdef CONFIG_X86_64 >>>> return this_cpu_read_stable(cpu_tss.x86_tss.sp0); >>>> #else >>>> /* sp0 on x86_32 is special in and around vm86 mode. */ >>>> return this_cpu_read_stable(cpu_current_top_of_stack); >>>> #endif >>>> } >>> >>> So can we then have: >>> >>> #ifdef __ASSEMBLY__ >>> # define cpu_current_top_of_stack (cpu_tss + TSS_sp0) >>> #else >>> # define cpu_current_top_of_stack (cpu_tss.x86_tss.sp0) >>> #endif >>> >>> And get rid of that if statement in the static inline? >> >> I prefer less macro indirection in assembly code, >> but I won't object too strongly if this would be done. >> >> It's up to other x86 maintainers to agree - I'm touching >> the code recently changed by Andy, he may see the way forward >> somewhat differently. >> > > I kind of like Steven's idea. Although.. shouldn't we go all the way and do: > > #ifdef __ASSEMBLY__ > #ifdef CONFIG_X86_64 > #define cpu_current_top_of_stack %gs:(cpu_tss + TSS_sp0) > #else > #define cpu_current_top_of_stack %gs:(current_top_of_stack) > #endif > #endif
Which .h file do you propose to have this in? processor.h is not suitable, it is not __ASSEMBLY__-fied.
I'm looking at a place where to put it. For example, one of the users of the (cpu_tss + TSS_sp0) expression, xen-asm_64.S, has only these few includes:
#include <asm/errno.h> #include <asm/percpu.h> #include <asm/processor-flags.h> #include <asm/segment.h> #include <asm/asm-offsets.h>
#include <xen/interface/xen.h>
#include "xen-asm.h"
None seems suitable. I will probably have to include thread_info.h too, and put the define there. Ugly ugly ugly.
I'll resend a patchset where a new patch #3 attempts to do this "unification". I personally don't see it as a clear improvement.
| |