Messages in this thread | | | Date | Tue, 13 May 2014 15:07:38 +0900 | From | Masami Hiramatsu <> | Subject | Re: can't we kill DIE_GPF ? (Was: x86/traps: cleanup DO_ERROR*() to prepare for uprobes fixes) |
| |
(2014/05/09 23:07), Oleg Nesterov wrote: > On 05/08, Oleg Nesterov wrote: >> >> For example, after this series >> we can convert math_error() into the "normal" DO_ERROR() user, and most probably >> we can do the same with do_general_protection(). > > As for do_general_protection(), the problem is DIE_GPF. > > Masami, could you explain why it is needed ? kprobe_exceptions_notify() > is the only user, can't it use DIE_TRAP and check trapnr = X86_TRAP_GP ?
Actually, this may be only for something which will happen on single-stepping out-of-line. And yes, I can move it onto the DIE_TRAP :)
> > And if it can, probably we can do notify_die() at the start like other > DO_ERROR() functions do ?
Agreed, it seems OK to me. (and seems better, since we can handle GPF before changing task->thread struct)
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Thanks a lot!
> > IOW, any reason why the patch below is wrong? > > Oleg. > > --- x/arch/x86/include/asm/kdebug.h > +++ x/arch/x86/include/asm/kdebug.h > @@ -15,7 +15,6 @@ enum die_val { > DIE_DIE, > DIE_KERNELDEBUG, > DIE_TRAP, > - DIE_GPF, > DIE_CALL, > DIE_PAGE_FAULT, > DIE_NMIUNKNOWN, > --- x/arch/x86/kernel/traps.c > +++ x/arch/x86/kernel/traps.c > @@ -283,6 +283,9 @@ do_general_protection(struct pt_regs *re > enum ctx_state prev_state; > > prev_state = exception_enter(); > + if (notify_die(DIE_TRAP, "general protection fault", regs, error_code, > + X86_TRAP_GP, SIGSEGV) == NOTIFY_STOP) > + goto exit; > conditional_sti(regs); > > #ifdef CONFIG_X86_32 > @@ -300,10 +303,7 @@ do_general_protection(struct pt_regs *re > > tsk->thread.error_code = error_code; > tsk->thread.trap_nr = X86_TRAP_GP; > - if (notify_die(DIE_GPF, "general protection fault", regs, error_code, > - X86_TRAP_GP, SIGSEGV) != NOTIFY_STOP) > - die("general protection fault", regs, error_code); > - goto exit; > + die("general protection fault", regs, error_code); > } > > tsk->thread.error_code = error_code; > --- x/arch/x86/kernel/kprobes/core.c > +++ x/arch/x86/kernel/kprobes/core.c > @@ -979,13 +979,14 @@ kprobe_exceptions_notify(struct notifier > ret = NOTIFY_STOP; > } > break; > - case DIE_GPF: > + case DIE_TRAP: > /* > * To be potentially processing a kprobe fault and to > * trust the result from kprobe_running(), we have > * be non-preemptible. > */ > - if (!preemptible() && kprobe_running() && > + if (args->trapnr == X86_TRAP_GP && > + !preemptible() && kprobe_running() && > kprobe_fault_handler(args->regs, args->trapnr)) > ret = NOTIFY_STOP; > break; > > -- > 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/ >
-- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com
| |