Re: [PATCH] x86: Reduce duplicated code in the x86_64 context switchpath.

From: Ingo Molnar
Date: Mon May 13 2013 - 13:30:59 EST



* Guenter Roeck <linux@xxxxxxxxxxxx> wrote:

> On Sun, May 12, 2013 at 10:14:51PM -0700, Joe Damato wrote:
> > Signed-off-by: Joe Damato <ice799@xxxxxxxxx>
> > ---
> > arch/x86/include/asm/switch_to.h | 30 ++++++++++++++++++++++++++++++
> > arch/x86/kernel/process_64.c | 29 ++---------------------------
> > 2 files changed, 32 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h
> > index 4ec45b3..a322cc6 100644
> > --- a/arch/x86/include/asm/switch_to.h
> > +++ b/arch/x86/include/asm/switch_to.h
> > @@ -124,6 +124,36 @@ do { \
> > __switch_canary_iparam \
> > : "memory", "cc" __EXTRA_CLOBBER)
> >
> > +#define loadsegment_fs(fs, index) \
> > + loadsegment(fs, index)
> > +
> > +#define loadsegment_gs(gs, index) \
> > + load_gs_index(index)
> > +
> > +#define switch_segment(prev, next, index, seg, msr) \
> > + do { \
> > + /* \
> > + * Segment register != 0 always requires a reload. Also \
> > + * reload when it has changed. When prev process used 64bit \
> > + * base always reload to avoid an information leak. \
> > + */ \
> > + if (unlikely(index | next->index | prev->seg)) { \
> > + loadsegment_##seg(seg, next->index); \
> > + /* \
> > + * Check if the user used a selector != 0; if yes \
> > + * clear 64bit base, since overloaded base is always \
> > + * mapped to the Null selector \
> > + */ \
> > + if (index) \
> > + prev->seg = 0; \
> > + } \
> > + \
> > + /* when next process has a 64bit base use it */ \
> > + if (next->seg) \
> > + wrmsrl(msr, next->seg); \
> > + prev->index = index; \
> > + } while (0)
> > +
> > #endif /* CONFIG_X86_32 */
> >
> For my part I'll never understand how code written as macros is supposed to
> improve anything. I always find it confusing and risky, as it is very easy
> to introduce side effects. Also, while it may reduce the source code size,
> it often results in increased object size.
>
> My take: If you can not write it as inline function(s), don't bother.

Indeed.

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/