All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>,
	Jim Keniston <jkenisto@us.ibm.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: can't we kill DIE_GPF ? (Was: x86/traps: cleanup DO_ERROR*() to prepare for uprobes fixes)
Date: Fri, 9 May 2014 16:07:23 +0200	[thread overview]
Message-ID: <20140509140723.GA22532@redhat.com> (raw)
In-Reply-To: <20140508191140.GA16892@redhat.com>

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 ?

And if it can, probably we can do notify_die() at the start like other
DO_ERROR() functions do ?

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;


  parent reply	other threads:[~2014-05-09 14:07 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-08 19:11 [PATCH 0/6] x86/traps: cleanup DO_ERROR*() to prepare for uprobes fixes Oleg Nesterov
2014-05-08 19:11 ` [PATCH 1/6] x86/traps: Make math_error() static Oleg Nesterov
2014-05-08 19:11 ` [PATCH 2/6] x86/traps: Use SEND_SIG_PRIV instead of force_sig() Oleg Nesterov
2014-05-08 19:11 ` [PATCH 3/6] x86/traps: Introduce do_error_trap() Oleg Nesterov
2014-05-08 19:12 ` [PATCH 4/6] x86/traps: Introduce fill_trap_info(), simplify DO_ERROR_INFO() Oleg Nesterov
2014-05-08 19:12 ` [PATCH 5/6] x86/traps: Shift fill_trap_info() from DO_ERROR_INFO() to do_error_trap() Oleg Nesterov
2014-05-08 19:12 ` [PATCH 6/6] x86/traps: Kill DO_ERROR_INFO() Oleg Nesterov
2014-05-09 14:07 ` Oleg Nesterov [this message]
2014-05-13  6:07   ` can't we kill DIE_GPF ? (Was: x86/traps: cleanup DO_ERROR*() to prepare for uprobes fixes) Masami Hiramatsu
2014-05-13 17:11     ` Oleg Nesterov
2014-05-12 17:08 ` [PATCH 0/1] (Was: " Oleg Nesterov
2014-05-12 17:08   ` [PATCH 1/1] uprobes/x86: Fix the wrong ->si_addr when xol triggers a trap Oleg Nesterov
2014-05-13  6:23     ` Masami Hiramatsu
2014-05-12 19:39   ` [PATCH 0/1] (Was: cleanup DO_ERROR*() to prepare for uprobes fixes) David Long
2014-05-13  5:10   ` Ananth N Mavinakayanahalli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140509140723.GA22532@redhat.com \
    --to=oleg@redhat.com \
    --cc=dvlasenk@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jkenisto@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@redhat.com \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.