On Mon, Feb 12, 2018 at 1:37 AM, Ingo Molnar <mi...@kernel.org> wrote:
>
> Ok, so this does not look _that_ complicated, and the .text savings are
> significant:

Honestly, I think we should do it. 3kB of assembly code is noticeable.

Also, that patch actually allows more cleanups and simplifications.
Look at the "interrupt" macro, which is used by 'apicinterrupt3', and
has a number of uses that way.

That code could be unified a lot, right now it does:

        testb   $3, CS-ORIG_RAX(%rsp)
        jz      1f
        SWAPGS
        call    switch_to_thread_stack
1:

        ALLOC_PT_GPREGS_ON_STACK
        SAVE_C_REGS
        SAVE_EXTRA_REGS
        ENCODE_FRAME_POINTER

        testb   $3, CS(%rsp)
        jz      1f

        /*
         * IRQ from user mode.
         *
         * We need to tell lockdep that IRQs are off.  We can't do this until
         * we fix gsbase, and we should do it before enter_from_user_mode
         * (which can take locks).  Since TRACE_IRQS_OFF idempotent,
         * the simplest way to handle it is to just call it twice if
         * we enter from user mode.  There's no reason to optimize this since
         * TRACE_IRQS_OFF is a no-op if lockdep is off.
         */
        TRACE_IRQS_OFF

        CALL_enter_from_user_mode

1:
        ENTER_IRQ_STACK old_rsp=%rdi
        /* We entered an interrupt context - irqs are off: */
        TRACE_IRQS_OFF

and *all* of that could be in a helper function rather than be
duplicated. and the apicinterrupt3 macro should end up just expanding
to

        callq helper
        pushq $~(\num)
        callq \fn
        jmp ret_from_intr

instead of expanding to all that code.

But that would require that same "save_ret" logic.

So it's not just the idtentry cases that can use this trick.

I admit that the trick isn't pretty, but it's not *horribly* ugly either.

                    Linus

Reply via email to