[RFC PATCH v2 -tip 0/4] x86: signal handler improvement - Kernel

This is a discussion on [RFC PATCH v2 -tip 0/4] x86: signal handler improvement - Kernel ; This patch series is experimental. This is a second version of the series. This update is for further testing. Changes v1->v2 - passing the error variable as a reference on __{put|get}_user_cerr. ======== I noticed there are inefficient codes in x86 ...

+ Reply to Thread
Results 1 to 10 of 10

Thread: [RFC PATCH v2 -tip 0/4] x86: signal handler improvement

  1. [RFC PATCH v2 -tip 0/4] x86: signal handler improvement

    This patch series is experimental.
    This is a second version of the series. This update is for further testing.

    Changes v1->v2
    - passing the error variable as a reference on __{put|get}_user_cerr.

    ========
    I noticed there are inefficient codes in x86 signals.
    For example, disassembled 32-bit setup_sigcontext();

    0000007c :
    7c: 55 push %ebp
    7d: 89 e5 mov %esp,%ebp
    7f: 57 push %edi
    80: 31 ff xor %edi,%edi
    82: 56 push %esi
    83: 89 c6 mov %eax,%esi
    85: 53 push %ebx
    86: 83 ec 58 sub $0x58,%esp
    89: 89 55 a4 mov %edx,-0x5c(%ebp)
    8c: 89 fa mov %edi,%edx
    8e: 8b 41 24 mov 0x24(%ecx),%eax
    91: 89 46 04 mov %eax,0x4(%esi)
    94: 89 55 a8 mov %edx,-0x58(%ebp)
    ....
    184: 8b 5d ac mov -0x54(%ebp),%ebx
    187: 0b 5d a8 or -0x58(%ebp),%ebx
    18a: 0b 5d b0 or -0x50(%ebp),%ebx
    18d: 0b 5d b4 or -0x4c(%ebp),%ebx
    190: 0b 5d b8 or -0x48(%ebp),%ebx
    193: 0b 5d bc or -0x44(%ebp),%ebx
    196: 0b 5d c0 or -0x40(%ebp),%ebx
    199: 0b 5d c4 or -0x3c(%ebp),%ebx
    19c: 0b 5d c8 or -0x38(%ebp),%ebx
    19f: 0b 5d cc or -0x34(%ebp),%ebx
    1a2: 0b 5d d0 or -0x30(%ebp),%ebx
    1a5: 0b 5d d4 or -0x2c(%ebp),%ebx
    1a8: 0b 5d d8 or -0x28(%ebp),%ebx
    1ab: 0b 5d dc or -0x24(%ebp),%ebx
    1ae: 0b 5d e0 or -0x20(%ebp),%ebx
    1b1: 0b 5d e4 or -0x1c(%ebp),%ebx
    1b4: 0b 5d e8 or -0x18(%ebp),%ebx
    1b7: 0b 5d ec or -0x14(%ebp),%ebx
    1ba: 0b 5d f0 or -0x10(%ebp),%ebx
    1bd: 09 fb or %edi,%ebx
    ....
    1dc: 09 d8 or %ebx,%eax
    1de: 5b pop %ebx
    1df: 09 c8 or %ecx,%eax
    1e1: 5e pop %esi
    1e2: 5f pop %edi
    1e3: 5d pop %ebp
    1e4: c3 ret

    there is a lot of "or" operation with stack, and it came from a set of
    following lines;

    err |= __put_user(x, ptr);

    The above line compiled to like this;
    a0: 89 fa mov %edi,%edx
    a2: 8b 41 20 mov 0x20(%ecx),%eax
    a5: 89 46 08 mov %eax,0x8(%esi)
    a8: 89 55 b0 mov %edx,-0x50(%ebp)

    and errors in __put_user is stored to stack.
    At the end of function, all errors in stack are calculated. If access to user
    space is failed, %edx is set to -EFAULT in exception handler and stored to
    stack for later operation.
    It seems inefficient.

    This patch series introduce __{put|get}_user_cerr for cumulative error
    handling, passing &err to cumulate errors. So, the line;
    err |= __put_user(x, ptr);
    changes to
    __put_user_cerr(x, ptr, &err);

    and it compiled to like this;
    92: 8b 41 20 mov 0x20(%ecx),%eax
    95: 89 47 08 mov %eax,0x8(%edi)

    The cumulative error is kept in the other register, in this example,
    %esi is used for this, and returns it. If access to user space causes fault,
    %esi is set to the value (%esi | -EFAULT) in exception handler.

    137: 89 f0 mov %esi,%eax
    139: 5b pop %ebx
    13a: 5e pop %esi
    13b: 5f pop %edi
    13c: 5d pop %ebp
    13d: c3 ret

    The results of this, I got a little performance improvement in signal
    handling. Here is a result of lmbench.
    I've tried 64-bit only at this time. Will measure on 32-bit.

    Processor, Processes - times in microseconds - smaller is better
    ------------------------------------------------------------------------------
    Host OS Mhz null null open slct sig sig fork exec sh
    call I/O stat clos TCP inst hndl proc proc proc
    --------- ------------- ---- ---- ---- ---- ---- ---- ---- ---- ---- ---- ----
    tip64-sig Linux 2.6.27- 3788 0.17 0.27 1.67 2.65 3.20 0.32 2.97 180. 524. 1806
    tip64-sig Linux 2.6.27- 3788 0.17 0.27 1.68 2.65 3.20 0.32 2.95 180. 520. 1796
    tip64-sig Linux 2.6.27- 3788 0.17 0.27 1.68 2.72 3.21 0.32 2.96 177. 528. 1812
    tip64 Linux 2.6.27- 3788 0.17 0.27 1.68 2.74 3.22 0.32 3.01 174. 523. 1853
    tip64 Linux 2.6.27- 3788 0.17 0.27 1.68 2.73 3.22 0.32 3.01 175. 523. 1806
    tip64 Linux 2.6.27- 3788 0.17 0.27 1.68 2.74 3.20 0.32 3.01 181. 523. 1791

    This patch series also reduce stack usages and code size.

    $ size signal_*
    text data bss dec hex filename
    4507 0 0 4507 119b signal_32.o.new
    5031 0 0 5031 13a7 signal_32.o.old
    3827 0 0 3827 ef3 signal_64.o.new
    4652 0 0 4652 122c signal_64.o.old

    Comments are welcome.
    I'll handle this patch series if it's good.

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

  2. [RFC PATCH v2 -tip 4/4] x86: signal: use __{put|get}_user_cerr

    From: Hiroshi Shimamoto

    Use __{put|get}_user_cerr for cumulative error handling in x86 signal code.
    This makes stack usage and code size small.

    The line
    err |= __put_user(x, ptr);
    is comiled to like this;
    a0: 89 fa mov %edi,%edx
    a2: 8b 41 20 mov 0x20(%ecx),%eax
    a5: 89 46 08 mov %eax,0x8(%esi)
    a8: 89 55 b0 mov %edx,-0x50(%ebp)

    and the line
    __put_user_cerr(x, ptr, &err);
    is comiled to like this;
    92: 8b 41 20 mov 0x20(%ecx),%eax
    95: 89 47 08 mov %eax,0x8(%edi)

    and the fixup code on exception looks like this;
    00000000 <.fixup>:
    0: 83 ce f2 or $0xfffffff2,%esi
    3: e9 8a 00 00 00 jmp 92 <.fixup+0x92>

    $ size signal_*
    text data bss dec hex filename
    4507 0 0 4507 119b signal_32.o.new
    5031 0 0 5031 13a7 signal_32.o.old
    3827 0 0 3827 ef3 signal_64.o.new
    4652 0 0 4652 122c signal_64.o.old

    Signed-off-by: Hiroshi Shimamoto
    ---
    arch/x86/kernel/signal_32.c | 96 +++++++++++++++++++++---------------------
    arch/x86/kernel/signal_64.c | 86 +++++++++++++++++++-------------------
    2 files changed, 91 insertions(+), 91 deletions(-)

    diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
    index 4337cd5..0c72f54 100644
    --- a/arch/x86/kernel/signal_32.c
    +++ b/arch/x86/kernel/signal_32.c
    @@ -126,21 +126,21 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc,
    /* Always make any pending restarted system calls return -EINTR */
    current_thread_info()->restart_block.fn = do_no_restart_syscall;

    -#define COPY(x) err |= __get_user(regs->x, &sc->x)
    +#define COPY(x) __get_user_cerr(regs->x, &sc->x, &err)

    #define COPY_SEG(seg) \
    { unsigned short tmp; \
    - err |= __get_user(tmp, &sc->seg); \
    + __get_user_cerr(tmp, &sc->seg, &err); \
    regs->seg = tmp; }

    #define COPY_SEG_STRICT(seg) \
    { unsigned short tmp; \
    - err |= __get_user(tmp, &sc->seg); \
    + __get_user_cerr(tmp, &sc->seg, &err); \
    regs->seg = tmp|3; }

    #define GET_SEG(seg) \
    { unsigned short tmp; \
    - err |= __get_user(tmp, &sc->seg); \
    + __get_user_cerr(tmp, &sc->seg, &err); \
    loadsegment(seg, tmp); }

    GET_SEG(gs);
    @@ -155,7 +155,7 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc,
    {
    unsigned int tmpflags;

    - err |= __get_user(tmpflags, &sc->flags);
    + __get_user_cerr(tmpflags, &sc->flags, &err);
    regs->flags = (regs->flags & ~FIX_EFLAGS) |
    (tmpflags & FIX_EFLAGS);
    regs->orig_ax = -1; /* disable syscall checks */
    @@ -164,11 +164,11 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc,
    {
    void __user *buf;

    - err |= __get_user(buf, &sc->fpstate);
    + __get_user_cerr(buf, &sc->fpstate, &err);
    err |= restore_i387_xstate(buf);
    }

    - err |= __get_user(*pax, &sc->ax);
    + __get_user_cerr(*pax, &sc->ax, &err);
    return err;
    }

    @@ -262,37 +262,37 @@ setup_sigcontext(struct sigcontext __user *sc, void __user *fpstate,
    {
    int tmp, err = 0;

    - err |= __put_user(regs->fs, (unsigned int __user *)&sc->fs);
    + __put_user_cerr(regs->fs, (unsigned int __user *)&sc->fs, &err);
    savesegment(gs, tmp);
    - err |= __put_user(tmp, (unsigned int __user *)&sc->gs);
    -
    - err |= __put_user(regs->es, (unsigned int __user *)&sc->es);
    - err |= __put_user(regs->ds, (unsigned int __user *)&sc->ds);
    - err |= __put_user(regs->di, &sc->di);
    - err |= __put_user(regs->si, &sc->si);
    - err |= __put_user(regs->bp, &sc->bp);
    - err |= __put_user(regs->sp, &sc->sp);
    - err |= __put_user(regs->bx, &sc->bx);
    - err |= __put_user(regs->dx, &sc->dx);
    - err |= __put_user(regs->cx, &sc->cx);
    - err |= __put_user(regs->ax, &sc->ax);
    - err |= __put_user(current->thread.trap_no, &sc->trapno);
    - err |= __put_user(current->thread.error_code, &sc->err);
    - err |= __put_user(regs->ip, &sc->ip);
    - err |= __put_user(regs->cs, (unsigned int __user *)&sc->cs);
    - err |= __put_user(regs->flags, &sc->flags);
    - err |= __put_user(regs->sp, &sc->sp_at_signal);
    - err |= __put_user(regs->ss, (unsigned int __user *)&sc->ss);
    + __put_user_cerr(tmp, (unsigned int __user *)&sc->gs, &err);
    +
    + __put_user_cerr(regs->es, (unsigned int __user *)&sc->es, &err);
    + __put_user_cerr(regs->ds, (unsigned int __user *)&sc->ds, &err);
    + __put_user_cerr(regs->di, &sc->di, &err);
    + __put_user_cerr(regs->si, &sc->si, &err);
    + __put_user_cerr(regs->bp, &sc->bp, &err);
    + __put_user_cerr(regs->sp, &sc->sp, &err);
    + __put_user_cerr(regs->bx, &sc->bx, &err);
    + __put_user_cerr(regs->dx, &sc->dx, &err);
    + __put_user_cerr(regs->cx, &sc->cx, &err);
    + __put_user_cerr(regs->ax, &sc->ax, &err);
    + __put_user_cerr(current->thread.trap_no, &sc->trapno, &err);
    + __put_user_cerr(current->thread.error_code, &sc->err, &err);
    + __put_user_cerr(regs->ip, &sc->ip, &err);
    + __put_user_cerr(regs->cs, (unsigned int __user *)&sc->cs, &err);
    + __put_user_cerr(regs->flags, &sc->flags, &err);
    + __put_user_cerr(regs->sp, &sc->sp_at_signal, &err);
    + __put_user_cerr(regs->ss, (unsigned int __user *)&sc->ss, &err);

    tmp = save_i387_xstate(fpstate);
    if (tmp < 0)
    err = 1;
    else
    - err |= __put_user(tmp ? fpstate : NULL, &sc->fpstate);
    + __put_user_cerr(tmp ? fpstate : NULL, &sc->fpstate, &err);

    /* non-iBCS2 extensions.. */
    - err |= __put_user(mask, &sc->oldmask);
    - err |= __put_user(current->thread.cr2, &sc->cr2);
    + __put_user_cerr(mask, &sc->oldmask, &err);
    + __put_user_cerr(current->thread.cr2, &sc->cr2, &err);

    return err;
    }
    @@ -377,7 +377,7 @@ __setup_frame(int sig, struct k_sigaction *ka, sigset_t *set,
    restorer = ka->sa.sa_restorer;

    /* Set up to return from userspace. */
    - err |= __put_user(restorer, &frame->pretcode);
    + __put_user_cerr(restorer, &frame->pretcode, &err);

    /*
    * This is popl %eax ; movl $__NR_sigreturn, %eax ; int $0x80
    @@ -386,9 +386,9 @@ __setup_frame(int sig, struct k_sigaction *ka, sigset_t *set,
    * reasons and because gdb uses it as a signature to notice
    * signal handler stack frames.
    */
    - err |= __put_user(0xb858, (short __user *)(frame->retcode+0));
    - err |= __put_user(__NR_sigreturn, (int __user *)(frame->retcode+2));
    - err |= __put_user(0x80cd, (short __user *)(frame->retcode+6));
    + __put_user_cerr(0xb858, (short __user *)(frame->retcode+0), &err);
    + __put_user_cerr(__NR_sigreturn, (int __user *)(frame->retcode+2), &err);
    + __put_user_cerr(0x80cd, (short __user *)(frame->retcode+6), &err);

    if (err)
    return -EFAULT;
    @@ -421,23 +421,23 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
    if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
    return -EFAULT;

    - err |= __put_user(sig, &frame->sig);
    - err |= __put_user(&frame->info, &frame->pinfo);
    - err |= __put_user(&frame->uc, &frame->puc);
    + __put_user_cerr(sig, &frame->sig, &err);
    + __put_user_cerr(&frame->info, &frame->pinfo, &err);
    + __put_user_cerr(&frame->uc, &frame->puc, &err);
    err |= copy_siginfo_to_user(&frame->info, info);
    if (err)
    return -EFAULT;

    /* Create the ucontext. */
    if (cpu_has_xsave)
    - err |= __put_user(UC_FP_XSTATE, &frame->uc.uc_flags);
    + __put_user_cerr(UC_FP_XSTATE, &frame->uc.uc_flags, &err);
    else
    - err |= __put_user(0, &frame->uc.uc_flags);
    - err |= __put_user(0, &frame->uc.uc_link);
    - err |= __put_user(current->sas_ss_sp, &frame->uc.uc_stack.ss_sp);
    - err |= __put_user(sas_ss_flags(regs->sp),
    - &frame->uc.uc_stack.ss_flags);
    - err |= __put_user(current->sas_ss_size, &frame->uc.uc_stack.ss_size);
    + __put_user_cerr(0, &frame->uc.uc_flags, &err);
    + __put_user_cerr(0, &frame->uc.uc_link, &err);
    + __put_user_cerr(current->sas_ss_sp, &frame->uc.uc_stack.ss_sp, &err);
    + __put_user_cerr(sas_ss_flags(regs->sp),
    + &frame->uc.uc_stack.ss_flags, &err);
    + __put_user_cerr(current->sas_ss_size, &frame->uc.uc_stack.ss_size, &err);
    err |= setup_sigcontext(&frame->uc.uc_mcontext, fpstate,
    regs, set->sig[0]);
    err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
    @@ -448,7 +448,7 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
    restorer = VDSO32_SYMBOL(current->mm->context.vdso, rt_sigreturn);
    if (ka->sa.sa_flags & SA_RESTORER)
    restorer = ka->sa.sa_restorer;
    - err |= __put_user(restorer, &frame->pretcode);
    + __put_user_cerr(restorer, &frame->pretcode, &err);

    /*
    * This is movl $__NR_rt_sigreturn, %ax ; int $0x80
    @@ -457,9 +457,9 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
    * reasons and because gdb uses it as a signature to notice
    * signal handler stack frames.
    */
    - err |= __put_user(0xb8, (char __user *)(frame->retcode+0));
    - err |= __put_user(__NR_rt_sigreturn, (int __user *)(frame->retcode+1));
    - err |= __put_user(0x80cd, (short __user *)(frame->retcode+5));
    + __put_user_cerr(0xb8, (char __user *)(frame->retcode+0), &err);
    + __put_user_cerr(__NR_rt_sigreturn, (int __user *)(frame->retcode+1), &err);
    + __put_user_cerr(0x80cd, (short __user *)(frame->retcode+5), &err);

    if (err)
    return -EFAULT;
    diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
    index 53f86d9..2d0f220 100644
    --- a/arch/x86/kernel/signal_64.c
    +++ b/arch/x86/kernel/signal_64.c
    @@ -64,7 +64,7 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc,
    /* Always make any pending restarted system calls return -EINTR */
    current_thread_info()->restart_block.fn = do_no_restart_syscall;

    -#define COPY(x) (err |= __get_user(regs->x, &sc->x))
    +#define COPY(x) __get_user_cerr(regs->x, &sc->x, &err)

    COPY(di); COPY(si); COPY(bp); COPY(sp); COPY(bx);
    COPY(dx); COPY(cx); COPY(ip);
    @@ -82,13 +82,13 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc,
    * App's signal handler can save/restore other segments if needed. */
    {
    unsigned cs;
    - err |= __get_user(cs, &sc->cs);
    + __get_user_cerr(cs, &sc->cs, &err);
    regs->cs = cs | 3; /* Force into user mode */
    }

    {
    unsigned int tmpflags;
    - err |= __get_user(tmpflags, &sc->flags);
    + __get_user_cerr(tmpflags, &sc->flags, &err);
    regs->flags = (regs->flags & ~FIX_EFLAGS) | (tmpflags & FIX_EFLAGS);
    regs->orig_ax = -1; /* disable syscall checks */
    }
    @@ -96,11 +96,11 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc,
    {
    void __user *buf;

    - err |= __get_user(buf, &sc->fpstate);
    + __get_user_cerr(buf, &sc->fpstate, &err);
    err |= restore_i387_xstate(buf);
    }

    - err |= __get_user(*pax, &sc->ax);
    + __get_user_cerr(*pax, &sc->ax, &err);
    return err;
    }

    @@ -150,32 +150,32 @@ setup_sigcontext(struct sigcontext __user *sc, struct pt_regs *regs,
    {
    int err = 0;

    - err |= __put_user(regs->cs, &sc->cs);
    - err |= __put_user(0, &sc->gs);
    - err |= __put_user(0, &sc->fs);
    -
    - err |= __put_user(regs->di, &sc->di);
    - err |= __put_user(regs->si, &sc->si);
    - err |= __put_user(regs->bp, &sc->bp);
    - err |= __put_user(regs->sp, &sc->sp);
    - err |= __put_user(regs->bx, &sc->bx);
    - err |= __put_user(regs->dx, &sc->dx);
    - err |= __put_user(regs->cx, &sc->cx);
    - err |= __put_user(regs->ax, &sc->ax);
    - err |= __put_user(regs->r8, &sc->r8);
    - err |= __put_user(regs->r9, &sc->r9);
    - err |= __put_user(regs->r10, &sc->r10);
    - err |= __put_user(regs->r11, &sc->r11);
    - err |= __put_user(regs->r12, &sc->r12);
    - err |= __put_user(regs->r13, &sc->r13);
    - err |= __put_user(regs->r14, &sc->r14);
    - err |= __put_user(regs->r15, &sc->r15);
    - err |= __put_user(me->thread.trap_no, &sc->trapno);
    - err |= __put_user(me->thread.error_code, &sc->err);
    - err |= __put_user(regs->ip, &sc->ip);
    - err |= __put_user(regs->flags, &sc->flags);
    - err |= __put_user(mask, &sc->oldmask);
    - err |= __put_user(me->thread.cr2, &sc->cr2);
    + __put_user_cerr(regs->cs, &sc->cs, &err);
    + __put_user_cerr(0, &sc->gs, &err);
    + __put_user_cerr(0, &sc->fs, &err);
    +
    + __put_user_cerr(regs->di, &sc->di, &err);
    + __put_user_cerr(regs->si, &sc->si, &err);
    + __put_user_cerr(regs->bp, &sc->bp, &err);
    + __put_user_cerr(regs->sp, &sc->sp, &err);
    + __put_user_cerr(regs->bx, &sc->bx, &err);
    + __put_user_cerr(regs->dx, &sc->dx, &err);
    + __put_user_cerr(regs->cx, &sc->cx, &err);
    + __put_user_cerr(regs->ax, &sc->ax, &err);
    + __put_user_cerr(regs->r8, &sc->r8, &err);
    + __put_user_cerr(regs->r9, &sc->r9, &err);
    + __put_user_cerr(regs->r10, &sc->r10, &err);
    + __put_user_cerr(regs->r11, &sc->r11, &err);
    + __put_user_cerr(regs->r12, &sc->r12, &err);
    + __put_user_cerr(regs->r13, &sc->r13, &err);
    + __put_user_cerr(regs->r14, &sc->r14, &err);
    + __put_user_cerr(regs->r15, &sc->r15, &err);
    + __put_user_cerr(me->thread.trap_no, &sc->trapno, &err);
    + __put_user_cerr(me->thread.error_code, &sc->err, &err);
    + __put_user_cerr(regs->ip, &sc->ip, &err);
    + __put_user_cerr(regs->flags, &sc->flags, &err);
    + __put_user_cerr(mask, &sc->oldmask, &err);
    + __put_user_cerr(me->thread.cr2, &sc->cr2, &err);

    return err;
    }
    @@ -229,19 +229,19 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,

    /* Create the ucontext. */
    if (cpu_has_xsave)
    - err |= __put_user(UC_FP_XSTATE, &frame->uc.uc_flags);
    + __put_user_cerr(UC_FP_XSTATE, &frame->uc.uc_flags, &err);
    else
    - err |= __put_user(0, &frame->uc.uc_flags);
    - err |= __put_user(0, &frame->uc.uc_link);
    - err |= __put_user(me->sas_ss_sp, &frame->uc.uc_stack.ss_sp);
    - err |= __put_user(sas_ss_flags(regs->sp),
    - &frame->uc.uc_stack.ss_flags);
    - err |= __put_user(me->sas_ss_size, &frame->uc.uc_stack.ss_size);
    - err |= setup_sigcontext(&frame->uc.uc_mcontext, regs, set->sig[0], me);
    - err |= __put_user(fp, &frame->uc.uc_mcontext.fpstate);
    + __put_user_cerr(0, &frame->uc.uc_flags, &err);
    + __put_user_cerr(0, &frame->uc.uc_link, &err);
    + __put_user_cerr(me->sas_ss_sp, &frame->uc.uc_stack.ss_sp, &err);
    + __put_user_cerr(sas_ss_flags(regs->sp),
    + &frame->uc.uc_stack.ss_flags, &err);
    + __put_user_cerr(me->sas_ss_size, &frame->uc.uc_stack.ss_size, &err);
    + setup_sigcontext(&frame->uc.uc_mcontext, regs, set->sig[0], me);
    + __put_user_cerr(fp, &frame->uc.uc_mcontext.fpstate, &err);
    if (sizeof(*set) == 16) {
    - __put_user(set->sig[0], &frame->uc.uc_sigmask.sig[0]);
    - __put_user(set->sig[1], &frame->uc.uc_sigmask.sig[1]);
    + __put_user_cerr(set->sig[0], &frame->uc.uc_sigmask.sig[0], &err);
    + __put_user_cerr(set->sig[1], &frame->uc.uc_sigmask.sig[1], &err);
    } else
    err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));

    @@ -249,7 +249,7 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
    already in userspace. */
    /* x86-64 should always use SA_RESTORER. */
    if (ka->sa.sa_flags & SA_RESTORER) {
    - err |= __put_user(ka->sa.sa_restorer, &frame->pretcode);
    + __put_user_cerr(ka->sa.sa_restorer, &frame->pretcode, &err);
    } else {
    /* could use a vstub here */
    return -EFAULT;
    --
    1.5.6

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

  3. [RFC PATCH v2 -tip 1/4] x86: uaccess: rename __put_user_u64 to __put_user_asm_u64

    From: Hiroshi Shimamoto

    Use same rule to name of __put_user and __get_user.

    Signed-off-by: Hiroshi Shimamoto
    ---
    include/asm-x86/uaccess.h | 6 +++---
    1 files changed, 3 insertions(+), 3 deletions(-)

    diff --git a/include/asm-x86/uaccess.h b/include/asm-x86/uaccess.h
    index 414b116..c098dfe 100644
    --- a/include/asm-x86/uaccess.h
    +++ b/include/asm-x86/uaccess.h
    @@ -186,7 +186,7 @@ extern int __get_user_bad(void);


    #ifdef CONFIG_X86_32
    -#define __put_user_u64(x, addr, err) \
    +#define __put_user_asm_u64(x, addr, err) \
    asm volatile("1: movl %%eax,0(%2)\n" \
    "2: movl %%edx,4(%2)\n" \
    "3:\n" \
    @@ -203,7 +203,7 @@ extern int __get_user_bad(void);
    asm volatile("call __put_user_8" : "=a" (__ret_pu) \
    : "A" ((typeof(*(ptr)))(x)), "c" (ptr) : "ebx")
    #else
    -#define __put_user_u64(x, ptr, retval) \
    +#define __put_user_asm_u64(x, ptr, retval) \
    __put_user_asm(x, ptr, retval, "q", "", "Zr", -EFAULT)
    #define __put_user_x8(x, ptr, __ret_pu) __put_user_x(8, x, ptr, __ret_pu)
    #endif
    @@ -279,7 +279,7 @@ do { \
    __put_user_asm(x, ptr, retval, "l", "k", "ir", errret);\
    break; \
    case 8: \
    - __put_user_u64((__typeof__(*ptr))(x), ptr, retval); \
    + __put_user_asm_u64((__typeof__(*ptr))(x), ptr, retval); \
    break; \
    default: \
    __put_user_bad(); \
    --
    1.5.6

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

  4. [RFC PATCH v2 -tip 3/4] x86: uaccess: introduce __{put|get}_user_cerr

    From: Hiroshi Shimamoto

    Introduce __{put|get}_user_cerr for cumulative error handling.
    The following 2 lines are same.
    __{put|get}_user_cerr(x, ptr, &err);
    err |= __{put|get}_user(x, ptr);

    Introduce __{put|get}_user_size_cerr for internal use from __{put|get}_user_cerr.

    Signed-off-by: Hiroshi Shimamoto
    ---
    include/asm-x86/uaccess.h | 74 +++++++++++++++++++++++++++++++++++++++++++++
    1 files changed, 74 insertions(+), 0 deletions(-)

    diff --git a/include/asm-x86/uaccess.h b/include/asm-x86/uaccess.h
    index 84b0600..284c7d3 100644
    --- a/include/asm-x86/uaccess.h
    +++ b/include/asm-x86/uaccess.h
    @@ -291,6 +291,31 @@ do { \
    } \
    } while (0)

    +#define __put_user_size_cerr(x, ptr, size, retval, errret) \
    +do { \
    + __chk_user_ptr(ptr); \
    + switch (size) { \
    + case 1: \
    + __put_user_asm_eop(x, ptr, retval, "b", "b", "iq", \
    + "or", errret); \
    + break; \
    + case 2: \
    + __put_user_asm_eop(x, ptr, retval, "w", "w", "ir", \
    + "or", errret); \
    + break; \
    + case 4: \
    + __put_user_asm_eop(x, ptr, retval, "l", "k", "ir", \
    + "or", errret); \
    + break; \
    + case 8: \
    + __put_user_asm_eop_u64((__typeof__(*ptr))(x), ptr, \
    + "or", retval); \
    + break; \
    + default: \
    + __put_user_bad(); \
    + } \
    +} while (0)
    +
    #else

    #define __put_user_size(x, ptr, size, retval, errret) \
    @@ -302,6 +327,14 @@ do { \
    retval = errret; \
    } while (0)

    +#define __put_user_size_cerr(x, ptr, size, retval, errret) \
    +do { \
    + __typeof__(*(ptr))__pus_tmp = x; \
    + \
    + if (unlikely(__copy_to_user_ll(ptr, &__pus_tmp, size) != 0)) \
    + retval |= errret; \
    +} while (0)
    +
    #define put_user(x, ptr) \
    ({ \
    int __ret_pu; \
    @@ -346,6 +379,30 @@ do { \
    } \
    } while (0)

    +#define __get_user_size_cerr(x, ptr, size, retval, errret) \
    +do { \
    + __chk_user_ptr(ptr); \
    + switch (size) { \
    + case 1: \
    + __get_user_asm_eop(x, ptr, retval, "b", "b", "=q", \
    + "or", errret); \
    + break; \
    + case 2: \
    + __get_user_asm_eop(x, ptr, retval, "w", "w", "=r", \
    + "or", errret); \
    + break; \
    + case 4: \
    + __get_user_asm_eop(x, ptr, retval, "l", "k", "=r", \
    + "or", errret); \
    + break; \
    + case 8: \
    + __get_user_asm_eop_u64(x, ptr, retval, "or", errret); \
    + break; \
    + default: \
    + (x) = __get_user_bad(); \
    + } \
    +} while (0)
    +
    #define __get_user_asm_eop(x, addr, err, itype, rtype, ltype, eop, errret) \
    asm volatile("1: mov"itype" %2,%"rtype"1\n" \
    "2:\n" \
    @@ -368,6 +425,11 @@ do { \
    __pu_err; \
    })

    +#define __put_user_nocheck_cerr(x, ptr, size, err) \
    +do { \
    + __put_user_size_cerr((x), (ptr), (size), *(err), -EFAULT); \
    +} while (0)
    +
    #define __get_user_nocheck(x, ptr, size) \
    ({ \
    long __gu_err; \
    @@ -377,6 +439,13 @@ do { \
    __gu_err; \
    })

    +#define __get_user_nocheck_cerr(x, ptr, size, err) \
    +do { \
    + unsigned long __gu_val; \
    + __get_user_size_cerr(__gu_val, (ptr), (size), *(err), -EFAULT); \
    + (x) = (__force __typeof__(*(ptr)))__gu_val; \
    +} while (0)
    +
    /* FIXME: this hack is definitely wrong -AK */
    struct __large_struct { unsigned long buf[100]; };
    #define __m(x) (*(struct __large_struct __user *)(x))
    @@ -423,6 +492,8 @@ struct __large_struct { unsigned long buf[100]; };

    #define __get_user(x, ptr) \
    __get_user_nocheck((x), (ptr), sizeof(*(ptr)))
    +#define __get_user_cerr(x, ptr, perr) \
    + __get_user_nocheck_cerr((x), (ptr), sizeof(*(ptr)), (perr))
    /**
    * __put_user: - Write a simple value into user space, with less checking.
    * @x: Value to copy to user space.
    @@ -445,6 +516,9 @@ struct __large_struct { unsigned long buf[100]; };

    #define __put_user(x, ptr) \
    __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
    +#define __put_user_cerr(x, ptr, perr) \
    + __put_user_nocheck_cerr((__typeof__(*(ptr)))(x), (ptr), \
    + sizeof(*(ptr)), (perr))

    #define __get_user_unaligned __get_user
    #define __put_user_unaligned __put_user
    --
    1.5.6

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

  5. [RFC PATCH v2 -tip 2/4] x86: uaccess: introduce __{put|get}_user_asm_eop

    From: Hiroshi Shimamoto

    Introduce __{put|get}_user_asm_eop which receives eop for error opecode.
    Define __{put|get}_user_asm as __{put|get}_user_asm_eop with "mov".

    Signed-off-by: Hiroshi Shimamoto
    ---
    include/asm-x86/uaccess.h | 27 +++++++++++++++++++++------
    1 files changed, 21 insertions(+), 6 deletions(-)

    diff --git a/include/asm-x86/uaccess.h b/include/asm-x86/uaccess.h
    index c098dfe..84b0600 100644
    --- a/include/asm-x86/uaccess.h
    +++ b/include/asm-x86/uaccess.h
    @@ -186,12 +186,12 @@ extern int __get_user_bad(void);


    #ifdef CONFIG_X86_32
    -#define __put_user_asm_u64(x, addr, err) \
    +#define __put_user_asm_eop_u64(x, addr, eop, err) \
    asm volatile("1: movl %%eax,0(%2)\n" \
    "2: movl %%edx,4(%2)\n" \
    "3:\n" \
    ".section .fixup,\"ax\"\n" \
    - "4: movl %3,%0\n" \
    + "4: " eop " %3,%0\n" \
    " jmp 3b\n" \
    ".previous\n" \
    _ASM_EXTABLE(1b, 4b) \
    @@ -199,12 +199,17 @@ extern int __get_user_bad(void);
    : "=r" (err) \
    : "A" (x), "r" (addr), "i" (-EFAULT), "0" (err))

    +#define __put_user_asm_u64(x, addr, err) \
    + __put_user_asm_eop_u64(x, addr, "movl", err)
    +
    #define __put_user_x8(x, ptr, __ret_pu) \
    asm volatile("call __put_user_8" : "=a" (__ret_pu) \
    : "A" ((typeof(*(ptr)))(x)), "c" (ptr) : "ebx")
    #else
    #define __put_user_asm_u64(x, ptr, retval) \
    __put_user_asm(x, ptr, retval, "q", "", "Zr", -EFAULT)
    +#define __put_user_asm_eop_u64(x, ptr, eop, retval) \
    + __put_user_asm_eop(x, ptr, retval, "q", "", "Zr", eop, -EFAULT)
    #define __put_user_x8(x, ptr, __ret_pu) __put_user_x(8, x, ptr, __ret_pu)
    #endif

    @@ -311,9 +316,12 @@ do { \

    #ifdef CONFIG_X86_32
    #define __get_user_asm_u64(x, ptr, retval, errret) (x) = __get_user_bad()
    +#define __get_user_asm_eop_u64(x, ptr, retval, eop, errret) (x) = __get_user_bad()
    #else
    #define __get_user_asm_u64(x, ptr, retval, errret) \
    __get_user_asm(x, ptr, retval, "q", "", "=r", errret)
    +#define __get_user_asm_eop_u64(x, ptr, retval, eop, errret) \
    + __get_user_asm_eop(x, ptr, retval, "q", "", "=r", eop, errret)
    #endif

    #define __get_user_size(x, ptr, size, retval, errret) \
    @@ -338,11 +346,11 @@ do { \
    } \
    } while (0)

    -#define __get_user_asm(x, addr, err, itype, rtype, ltype, errret) \
    +#define __get_user_asm_eop(x, addr, err, itype, rtype, ltype, eop, errret) \
    asm volatile("1: mov"itype" %2,%"rtype"1\n" \
    "2:\n" \
    ".section .fixup,\"ax\"\n" \
    - "3: mov %3,%0\n" \
    + "3: " eop " %3,%0\n" \
    " xor"itype" %"rtype"1,%"rtype"1\n" \
    " jmp 2b\n" \
    ".previous\n" \
    @@ -350,6 +358,9 @@ do { \
    : "=r" (err), ltype(x) \
    : "m" (__m(addr)), "i" (errret), "0" (err))

    +#define __get_user_asm(x, addr, err, itype, rtype, ltype, errret) \
    + __get_user_asm_eop(x, addr, err, itype, rtype, ltype, "mov", errret)
    +
    #define __put_user_nocheck(x, ptr, size) \
    ({ \
    long __pu_err; \
    @@ -375,16 +386,20 @@ struct __large_struct { unsigned long buf[100]; };
    * we do not write to any memory gcc knows about, so there are no
    * aliasing issues.
    */
    -#define __put_user_asm(x, addr, err, itype, rtype, ltype, errret) \
    +#define __put_user_asm_eop(x, addr, err, itype, rtype, ltype, eop, errret) \
    asm volatile("1: mov"itype" %"rtype"1,%2\n" \
    "2:\n" \
    ".section .fixup,\"ax\"\n" \
    - "3: mov %3,%0\n" \
    + "3: " eop " %3,%0\n" \
    " jmp 2b\n" \
    ".previous\n" \
    _ASM_EXTABLE(1b, 3b) \
    : "=r"(err) \
    : ltype(x), "m" (__m(addr)), "i" (errret), "0" (err))
    +
    +#define __put_user_asm(x, addr, err, itype, rtype, ltype, errret) \
    + __put_user_asm_eop(x, addr, err, itype, rtype, ltype, "mov", errret)
    +
    /**
    * __get_user: - Get a simple variable from user space, with less checking.
    * @x: Variable to store result.
    --
    1.5.6

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

  6. Re: [RFC PATCH v2 -tip 0/4] x86: signal handler improvement

    Hiroshi Shimamoto wrote:
    >
    > and errors in __put_user is stored to stack.
    > At the end of function, all errors in stack are calculated. If access to user
    > space is failed, %edx is set to -EFAULT in exception handler and stored to
    > stack for later operation.
    > It seems inefficient.
    >


    This is a gcc failure, and should be reported to the gcc people.

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

  7. Re: [RFC PATCH v2 -tip 0/4] x86: signal handler improvement

    H. Peter Anvin wrote:
    > Hiroshi Shimamoto wrote:
    >> and errors in __put_user is stored to stack.
    >> At the end of function, all errors in stack are calculated. If access to user
    >> space is failed, %edx is set to -EFAULT in exception handler and stored to
    >> stack for later operation.
    >> It seems inefficient.
    >>

    >
    > This is a gcc failure, and should be reported to the gcc people.


    Does this gcc failure mean unnecessary storing into stack?

    thanks,
    Hiroshi Shimamoto

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

  8. Re: [RFC PATCH v2 -tip 0/4] x86: signal handler improvement

    Hiroshi Shimamoto wrote:
    >>>

    >> This is a gcc failure, and should be reported to the gcc people.

    >
    > Does this gcc failure mean unnecessary storing into stack?
    >


    Yes, it should be able to process this in a register, instead of storing
    to the stack and then merging later.

    It's possible it's trying to do that to hide latency, but that's clearly
    a lose in this case.

    I'd hate to obfuscate the code, and I'd *really* hate to obfuscate the
    code to work around gcc brokenness, and then not even bothering to tell
    the gcc folks.

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

  9. Re: [RFC PATCH v2 -tip 0/4] x86: signal handler improvement

    H. Peter Anvin wrote:
    > Hiroshi Shimamoto wrote:
    >>> This is a gcc failure, and should be reported to the gcc people.

    >> Does this gcc failure mean unnecessary storing into stack?
    >>

    >
    > Yes, it should be able to process this in a register, instead of storing
    > to the stack and then merging later.
    >
    > It's possible it's trying to do that to hide latency, but that's clearly
    > a lose in this case.


    OK, I'll test with some gcc versions.
    It's necessary to check outputs of the latest gcc, I think.

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

  10. Re: [RFC PATCH v2 -tip 0/4] x86: signal handler improvement


    * H. Peter Anvin wrote:

    > Hiroshi Shimamoto wrote:
    >>>>
    >>> This is a gcc failure, and should be reported to the gcc people.

    >>
    >> Does this gcc failure mean unnecessary storing into stack?

    >
    > Yes, it should be able to process this in a register, instead of
    > storing to the stack and then merging later.
    >
    > It's possible it's trying to do that to hide latency, but that's
    > clearly a lose in this case.
    >
    > I'd hate to obfuscate the code, and I'd *really* hate to obfuscate the
    > code to work around gcc brokenness, and then not even bothering to
    > tell the gcc folks.


    assuming it's reported to the gcc folks too, do you have any objections
    against including this in tip/x86/signal? The 1% improvement in lmbench
    is quite nice.

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

+ Reply to Thread