[PATCH 0/7] traps: x86: irqtrace cleanup and some traps.c unification

Previous thread: none

Next thread: GFS2: Pre-pull patch posting by Steven Whitehouse on Friday, September 26, 2008 - 5:00 am. (13 messages)
From: Alexander van Heukelum
Date: Friday, September 26, 2008 - 5:03 am

Aparantly the conversion of exceptions from traps to interrupts
and the changes to the irq-tracer on i386 have not given any
trouble, yet? (tip/x86/traps)

This is a small set of patches removing trace_hardirqs_fixup
completely from the code. TRACE_IRQS_OFF is added to the assembly
code instead, where needed.

7/7 is a small unification step between traps_32.c and traps_64.c

Patches were generated against tip/x86/traps.

Greetings,
    Alexander

arch/x86/kernel/entry_64.S |    6 ++++-
arch/x86/kernel/traps_32.c |   55 ++++++++++++-------------------------------
arch/x86/kernel/traps_64.c |   11 ++------
arch/x86/mm/fault.c        |    5 ----
include/asm-x86/irqflags.h |   21 ----------------
5 files changed, 24 insertions(+), 74 deletions(-)
      
--

From: Alexander van Heukelum
Date: Friday, September 26, 2008 - 5:03 am

Add TRACE_IRQS_OFF just before entering the C code.

All exceptions are taken via interrupt gates. If irq tracing is
enabled, it should be notified as soon as possible. Interrupts
are only (conditionally) re-enabled in C code.

Signed-off-by: Alexander van Heukelum <fastmail.fm>
---
 arch/x86/kernel/entry_64.S |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 89434d4..78fa552 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1058,7 +1058,8 @@ KPROBE_ENTRY(error_entry)
 	je  error_kernelspace
 error_swapgs:	
 	SWAPGS
-error_sti:	
+error_sti:
+	TRACE_IRQS_OFF
 	movq %rdi,RDI(%rsp) 	
 	CFI_REL_OFFSET	rdi,RDI
 	movq %rsp,%rdi
-- 
1.5.4.3

--

From: Alexander van Heukelum
Date: Friday, September 26, 2008 - 5:03 am

Add TRACE_IRQS_OFF just before entering the C code.

All exceptions are taken via interrupt gates. If irq tracing is
enabled, it should be notified as soon as possible. Interrupts
are only (conditionally) re-enabled in C code.

Signed-off-by: Alexander van Heukelum <fastmail.fm>
---
 arch/x86/kernel/entry_64.S |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 78fa552..14ea704 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -932,6 +932,9 @@ END(spurious_interrupt)
 	.if \ist
 	movq	%gs:pda_data_offset, %rbp
 	.endif
+	.if \irqtrace
+	TRACE_IRQS_OFF
+	.endif
 	movq %rsp,%rdi
 	movq ORIG_RAX(%rsp),%rsi
 	movq $-1,ORIG_RAX(%rsp)
-- 
1.5.4.3

--

From: Alexander van Heukelum
Date: Friday, September 26, 2008 - 5:03 am

All exceptions are taken via interrupt gates. TRACE_IRQS_OFF
is called just before entering the C code, so the irq state
is known to the irq tracer at this point. No need to call
trace_hardirqs_fixup.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_64.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/traps_64.c b/arch/x86/kernel/traps_64.c
index 513caac..04b163e 100644
--- a/arch/x86/kernel/traps_64.c
+++ b/arch/x86/kernel/traps_64.c
@@ -655,7 +655,6 @@ asmlinkage void do_##name(struct pt_regs * regs, long error_code)	\
 	info.si_errno = 0;						\
 	info.si_code = sicode;						\
 	info.si_addr = (void __user *)siaddr;				\
-	trace_hardirqs_fixup();						\
 	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr)	\
 							== NOTIFY_STOP)	\
 		return;							\
-- 
1.5.4.3

--

From: Alexander van Heukelum
Date: Friday, September 26, 2008 - 5:03 am

All exceptions are taken via interrupt gates. TRACE_IRQS_OFF
is called just before entering the C code, so the irq state
is known to the irq tracer at this point. No need to call
trace_hardirqs_fixup.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_64.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/traps_64.c b/arch/x86/kernel/traps_64.c
index 04b163e..2ff759e 100644
--- a/arch/x86/kernel/traps_64.c
+++ b/arch/x86/kernel/traps_64.c
@@ -857,8 +857,6 @@ void restart_nmi(void)
 /* runs on IST stack. */
 asmlinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
 {
-	trace_hardirqs_fixup();
-
 	if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
 			== NOTIFY_STOP)
 		return;
-- 
1.5.4.3

--

From: Alexander van Heukelum
Date: Friday, September 26, 2008 - 5:03 am

All exceptions are taken via interrupt gates. TRACE_IRQS_OFF
is called just before entering the C code, so the irq state
is known to the irq tracer at this point. No need to call
trace_hardirqs_fixup.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_64.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/traps_64.c b/arch/x86/kernel/traps_64.c
index 2ff759e..c5fb747 100644
--- a/arch/x86/kernel/traps_64.c
+++ b/arch/x86/kernel/traps_64.c
@@ -895,8 +895,6 @@ asmlinkage void __kprobes do_debug(struct pt_regs * regs,
 	unsigned long condition;
 	siginfo_t info;
 
-	trace_hardirqs_fixup();
-
 	get_debugreg(condition, 6);
 
 	/*
-- 
1.5.4.3

--

From: Alexander van Heukelum
Date: Friday, September 26, 2008 - 5:03 am

The last use of trace_hardirqs_fixup is unnecessary, because the
trap is taken with interrupt off on i386 as well as x86_64, and
the irq-tracer is notified of this from the assembly code.

trace_hardirqs_fixup and trace_hardirqs_fixup_flags are removed
from include/asm-x86/irqflags.h as they are no longer used.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/mm/fault.c        |    5 -----
 include/asm-x86/irqflags.h |   21 ---------------------
 2 files changed, 0 insertions(+), 26 deletions(-)

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 455f3fe..c5bafc9 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -593,11 +593,6 @@ void __kprobes do_page_fault(struct pt_regs *regs, unsigned long error_code)
 	unsigned long flags;
 #endif
 
-	/*
-	 * We can fault from pretty much anywhere, with unknown IRQ state.
-	 */
-	trace_hardirqs_fixup();
-
 	tsk = current;
 	mm = tsk->mm;
 	prefetchw(&mm->mmap_sem);
diff --git a/include/asm-x86/irqflags.h b/include/asm-x86/irqflags.h
index 424acb4..2bdab21 100644
--- a/include/asm-x86/irqflags.h
+++ b/include/asm-x86/irqflags.h
@@ -166,27 +166,6 @@ static inline int raw_irqs_disabled(void)
 	return raw_irqs_disabled_flags(flags);
 }
 
-/*
- * makes the traced hardirq state match with the machine state
- *
- * should be a rarely used function, only in places where its
- * otherwise impossible to know the irq state, like in traps.
- */
-static inline void trace_hardirqs_fixup_flags(unsigned long flags)
-{
-	if (raw_irqs_disabled_flags(flags))
-		trace_hardirqs_off();
-	else
-		trace_hardirqs_on();
-}
-
-static inline void trace_hardirqs_fixup(void)
-{
-	unsigned long flags = __raw_local_save_flags();
-
-	trace_hardirqs_fixup_flags(flags);
-}
-
 #else
 
 #ifdef CONFIG_X86_64
-- 
1.5.4.3

--

From: Alexander van Heukelum
Date: Friday, September 26, 2008 - 5:03 am

This patch hardcodes which traps should be forwarded to
handle_vm86_trap in do_trap. This allows to remove the
vm86 parameter from the i386-version of do_trap, which
makes the DO_VM86_ERROR and DO_VM86_ERROR_INFO macros
unnecessary.

x86_64 part is whitespace only.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_32.c |   55 ++++++++++++-------------------------------
 arch/x86/kernel/traps_64.c |    6 ++--
 2 files changed, 19 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kernel/traps_32.c b/arch/x86/kernel/traps_32.c
index 041a25e..a603554 100644
--- a/arch/x86/kernel/traps_32.c
+++ b/arch/x86/kernel/traps_32.c
@@ -482,13 +482,17 @@ die_if_kernel(const char *str, struct pt_regs *regs, long err)
 }
 
 static void __kprobes
-do_trap(int trapnr, int signr, char *str, int vm86, struct pt_regs *regs,
+do_trap(int trapnr, int signr, char *str, struct pt_regs *regs,
 	long error_code, siginfo_t *info)
 {
 	struct task_struct *tsk = current;
 
 	if (regs->flags & X86_VM_MASK) {
-		if (vm86)
+		/*
+		 * traps 0, 1, 3, 4, and 5 should be forwarded to vm86.
+		 * On nmi (interrupt 2), do_trap should not be called.
+		 */
+		if (trapnr < 6)
 			goto vm86_trap;
 		goto trap_signal;
 	}
@@ -537,37 +541,10 @@ void do_##name(struct pt_regs *regs, long error_code)			\
 							== NOTIFY_STOP)	\
 		return;							\
 	conditional_sti(regs);						\
-	do_trap(trapnr, signr, str, 0, regs, error_code, NULL);		\
-}
-
-#define DO_ERROR_INFO(trapnr, signr, str, name, sicode, siaddr, irq)	\
-void do_##name(struct pt_regs *regs, long error_code)			\
-{									\
-	siginfo_t info;							\
-	if (irq)							\
-		local_irq_enable();					\
-	info.si_signo = signr;						\
-	info.si_errno = 0;						\
-	info.si_code = sicode;						\
-	info.si_addr = (void __user *)siaddr;				\
-	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr)	\
-							== NOTIFY_STOP)	\
-		return;							\
-	conditional_sti(regs);						\
-	do_trap(trapnr, ...
From: Alexander van Heukelum
Date: Friday, September 26, 2008 - 5:39 am

On Fri, 26 Sep 2008 14:03:02 +0200, "Alexander van Heukelum"

That should be: <heukelum@fastmail.fm> ---^^^
-- 
  Alexander van Heukelum
  heukelum@fastmail.fm

-- 
http://www.fastmail.fm - Or how I learned to stop worrying and
                          love email again

--

From: Ingo Molnar
Date: Saturday, September 27, 2008 - 9:30 am

Correct.

Generally you'll hear from the -tip maintainers if there's any trouble 
with a topic. If you see it integrated into tip/master and if you see no 


applied to tip/x86/traps, thanks.

	Ingo
--

Previous thread: none

Next thread: GFS2: Pre-pull patch posting by Steven Whitehouse on Friday, September 26, 2008 - 5:00 am. (13 messages)