lkml.org 
[lkml]   [2014]   [May]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: 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




\
 
 \ /
  Last update: 2014-05-13 08:41    [W:1.041 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site