Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1719964 > unrolled thread

[patch 00/41] x86: Cleanup IDT code

Back to article view | Back to linux.kernel

Contents

  [patch 00/41] x86: Cleanup IDT code Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 01/41] x86/irq: Remove vector_used_by_percpu_irq() Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 26/41] x86/idt: Consolidate IDT invalidation Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 06/41] x86/traps: Simplify pagefault tracing logic Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 06/41] x86/traps: Simplify pagefault tracing logic Andy Lutomirski <luto@kernel.org> - 2017-08-25 17:00 +0200
      Re: [patch 06/41] x86/traps: Simplify pagefault tracing logic Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 17:20 +0200
        Re: [patch 06/41] x86/traps: Simplify pagefault tracing logic Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 17:20 +0200
    [patch 07/41] x86/apic: Remove the duplicated tracing version of  local_timer_interrupt Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 07/41] x86/apic: Remove the duplicated tracing version  of local_timer_interrupt Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 17:30 +0200
    [patch 14/41] x86/smp: Use static key for reschedule interrupt tracing Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 14/41] x86/smp: Use static key for reschedule interrupt  tracing Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 21:50 +0200
        Re: [patch 14/41] x86/smp: Use static key for reschedule interrupt  tracing Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 22:10 +0200
          Re: [patch 14/41] x86/smp: Use static key for reschedule interrupt  tracing Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 23:20 +0200
    [patch 05/41] x86/tracing: Introduce a static key for exception  tracing Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 05/41] x86/tracing: Introduce a static key for exception  tracing Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 17:30 +0200
    [patch 10/41] x86/apic: Remove the duplicated tracing versions of  interrupts Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 10/41] x86/apic: Remove the duplicated tracing versions  of interrupts Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 17:50 +0200
        Re: [patch 10/41] x86/apic: Remove the duplicated tracing versions  of interrupts Peter Zijlstra <peterz@infradead.org> - 2017-08-28 10:00 +0200
    [patch 11/41] x86/irqwork: Get rid of duplicated tracing interrupt  code Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 11/41] x86/irqwork: Get rid of duplicated tracing  interrupt code Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 18:00 +0200
    [patch 23/41] x86/idt: Create file for IDT related code Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 03/41] x86/irq: Move ifdeffery to header file Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 03/41] x86/irq: Move ifdeffery to header file Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 14:50 +0200
    [patch 24/41] x86/idt: Move 32bit idt_descr to C code Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 16/41] x86/tracing: Disentangle pagefault and resched IPI  tracing key Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 09/41] x86/irq: Get rid of duplicated trace_x86_platform_ipi()  code Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 09/41] x86/irq: Get rid of duplicated  trace_x86_platform_ipi() code Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 17:50 +0200
    [patch 12/41] x86/mce: Remove duplicated tracing interrupt code Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 12/41] x86/mce: Remove duplicated tracing interrupt code Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 18:10 +0200
        Re: [patch 12/41] x86/mce: Remove duplicated tracing interrupt  code Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 18:20 +0200
    [patch 21/41] x86/gdt: Use bitfields for initialization Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 28/41] x86/idt: Move early IDT setup out of 32bit asm Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 22/41] x86/ldttss: Cleanup 32bit descriptors Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 20/41] x86: Replace access to desc_struct:a/b fields Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 20/41] x86: Replace access to desc_struct:a/b fields Juergen Gross <jgross@suse.com> - 2017-08-25 13:20 +0200
      Re: [patch 20/41] x86: Replace access to desc_struct:a/b fields Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-08-25 16:00 +0200
        Re: [patch 20/41] x86: Replace access to desc_struct:a/b fields Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 16:10 +0200
          Re: [patch 20/41] x86: Replace access to desc_struct:a/b fields Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 16:20 +0200
    [patch 04/41] x86/boot: Move EISA setup to a proper place Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 04/41] x86/boot: Move EISA setup to a proper place Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 14:00 +0200
    [patch 25/41] x86/idt: Remove unused set_trap_gate() Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 08/41] x86/apic: Use this_cpu_ptr in local_timer_interrupt Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
    [patch 15/41] x86/idt: Remove tracing idt completely Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 13:10 +0200
      Re: [patch 15/41] x86/idt: Remove tracing idt completely Steven Rostedt <rostedt@goodmis.org> - 2017-08-25 23:20 +0200
        Re: [patch 15/41] x86/idt: Remove tracing idt completely Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 23:30 +0200
        Re: [patch 15/41] x86/idt: Remove tracing idt completely Thomas Gleixner <tglx@linutronix.de> - 2017-08-25 23:30 +0200

Page 1 of 3  [1] 2 3  Next page →

#1719964 — [patch 00/41] x86: Cleanup IDT code

The IDT code in x86 is a convoluted mess and contains quite some
bogosities.

One of the main offenders is the extra tracing IDT which has been
introduced to reduce the impact of tracing on the exception vectors when
tracepoints are disabled.

This results in an unholy macro mess which magically creates IDT entries by
prepending the exception handler name by trace_.

For most exceptions the tracepoint overhead in the disabled case is two
NOP5, which are negligible compared to the heavy work of the exception
handler itself. Modern CPUs are pretty good in optimizing NOPs.

The only two handlers where the overhead matters are the pagefault handler
and the reschedule IPI, where an active tracepoint introduces more extra
work. These two can be handled with separate static keys which reduces the
impact to a minimum.

As a result the whole tracing IDT machinery can be removed.

The second part of the series cleans up the handling of IDT/GDT/LDT/TSS
descriptors to get rid of macro based initialization via the obscure raw
32bit union fields a and b.

The last part moves the setup of the IDT into a separate file, where all
gate related code is moved to. The gate setup is converted to a table based
mechanism so the resulting code is smaller and better understandable.

The series applies on top of

 git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/apic

and is available as a git branch from

 git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.x86/apic

There is more cleanup to come in the near future, which aims at replacing
the braindamaged vector assignment mechanics, which has quite some
shortcomings.

Thanks,

	tglx
---
 arch/x86/boot/compressed/eboot.c         |    8 
 arch/x86/entry/entry_32.S                |   20 -
 arch/x86/entry/entry_64.S                |   26 --
 arch/x86/entry/vdso/vma.c                |    2 
 arch/x86/include/asm/desc.h              |  246 +++-----------------
 arch/x86/include/asm/desc_defs.h         |  122 +++++-----
 arch/x86/include/asm/entry_arch.h        |   14 -
 arch/x86/include/asm/hw_irq.h            |   20 -
 arch/x86/include/asm/irq.h               |    4 
 arch/x86/include/asm/segment.h           |    4 
 arch/x86/include/asm/trace/exceptions.h  |    8 
 arch/x86/include/asm/trace/irq_vectors.h |   24 +-
 arch/x86/include/asm/traps.h             |   10 
 arch/x86/include/asm/xen/hypercall.h     |    6 
 arch/x86/kernel/Makefile                 |    3 
 arch/x86/kernel/apic/apic.c              |   70 +----
 arch/x86/kernel/apic/vector.c            |    2 
 arch/x86/kernel/cpu/common.c             |    9 
 arch/x86/kernel/cpu/mcheck/mce_amd.c     |   16 -
 arch/x86/kernel/cpu/mcheck/therm_throt.c |   20 -
 arch/x86/kernel/cpu/mcheck/threshold.c   |   16 -
 arch/x86/kernel/cpu/mshyperv.c           |    9 
 arch/x86/kernel/head32.c                 |    4 
 arch/x86/kernel/head64.c                 |    6 
 arch/x86/kernel/head_32.S                |   42 ---
 arch/x86/kernel/irq.c                    |   29 --
 arch/x86/kernel/irq_work.c               |   16 -
 arch/x86/kernel/irqinit.c                |  102 --------
 arch/x86/kernel/kvm.c                    |    4 
 arch/x86/kernel/machine_kexec_32.c       |   14 -
 arch/x86/kernel/reboot.c                 |    4 
 arch/x86/kernel/setup.c                  |    4 
 arch/x86/kernel/setup_percpu.c           |    9 
 arch/x86/kernel/smp.c                    |   81 +-----
 arch/x86/kernel/tls.c                    |    2 
 arch/x86/kernel/tracepoint.c             |   57 +---
 arch/x86/kernel/traps.c                  |  108 ---------
 arch/x86/kvm/vmx.c                       |    2 
 arch/x86/math-emu/fpu_entry.c            |   11 
 arch/x86/math-emu/fpu_system.h           |   48 +++-
 arch/x86/math-emu/get_address.c          |   17 -
 arch/x86/mm/fault.c                      |   47 +--
 arch/x86/xen/enlighten_pv.c              |   14 -
 b/arch/x86/include/asm/trace/common.h    |   16 +
 b/arch/x86/kernel/eisa.c                 |   18 +
 b/arch/x86/kernel/idt.c                  |  365 +++++++++++++++++++++++++++++++
 drivers/xen/events/events_base.c         |    6 
 47 files changed, 718 insertions(+), 967 deletions(-)

[toc] | [next] | [standalone]

#1719965 — [patch 01/41] x86/irq: Remove vector_used_by_percpu_irq()

Last user (lguest) is gone. Remove it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/irq.h |    1 -
 arch/x86/kernel/irq.c      |    2 --
 arch/x86/kernel/irqinit.c  |   12 ------------
 3 files changed, 15 deletions(-)

--- a/arch/x86/include/asm/irq.h
+++ b/arch/x86/include/asm/irq.h
@@ -44,7 +44,6 @@ extern __visible unsigned int do_IRQ(str
 
 /* Interrupt vector management */
 extern DECLARE_BITMAP(used_vectors, NR_VECTORS);
-extern int vector_used_by_percpu_irq(unsigned int vector);
 
 extern void init_ISA_irqs(void);
 
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -346,8 +346,6 @@ EXPORT_SYMBOL_GPL(kvm_set_posted_intr_wa
 	set_irq_regs(old_regs);
 }
 
-EXPORT_SYMBOL_GPL(vector_used_by_percpu_irq);
-
 #ifdef CONFIG_HOTPLUG_CPU
 
 /* These two declarations are only used in check_irq_vectors_for_cpu_disable()
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -55,18 +55,6 @@ DEFINE_PER_CPU(vector_irq_t, vector_irq)
 	[0 ... NR_VECTORS - 1] = VECTOR_UNUSED,
 };
 
-int vector_used_by_percpu_irq(unsigned int vector)
-{
-	int cpu;
-
-	for_each_online_cpu(cpu) {
-		if (!IS_ERR_OR_NULL(per_cpu(vector_irq, cpu)[vector]))
-			return 1;
-	}
-
-	return 0;
-}
-
 void __init init_ISA_irqs(void)
 {
 	struct irq_chip *chip = legacy_pic->chip;

[toc] | [prev] | [next] | [standalone]

#1719966 — [patch 26/41] x86/idt: Consolidate IDT invalidation

kexec and reboot have both code to invalidate IDT. Create a common function
and use it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/desc.h        |    3 +++
 arch/x86/kernel/idt.c              |   11 +++++++++++
 arch/x86/kernel/machine_kexec_32.c |   14 +-------------
 arch/x86/kernel/reboot.c           |    4 +---
 4 files changed, 16 insertions(+), 16 deletions(-)

--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -512,4 +512,7 @@ static inline void load_current_idt(void
 	else
 		load_idt((const struct desc_ptr *)&idt_descr);
 }
+
+extern void idt_invalidate(void *addr);
+
 #endif /* _ASM_X86_DESC_H */
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -24,3 +24,14 @@ const struct desc_ptr debug_idt_descr =
 	.address	= (unsigned long) debug_idt_table,
 };
 #endif
+
+/**
+ * idt_invalidate - Invalidate interrupt descriptor table
+ * @addr:	The virtual address of the 'invalid' IDT
+ */
+void idt_invalidate(void *addr)
+{
+	struct desc_ptr idt = { .address = (unsigned long) addr, .size = 0 };
+
+	load_idt(&idt);
+}
--- a/arch/x86/kernel/machine_kexec_32.c
+++ b/arch/x86/kernel/machine_kexec_32.c
@@ -26,18 +26,6 @@
 #include <asm/set_memory.h>
 #include <asm/debugreg.h>
 
-static void set_idt(void *newidt, __u16 limit)
-{
-	struct desc_ptr curidt;
-
-	/* ia32 supports unaliged loads & stores */
-	curidt.size    = limit;
-	curidt.address = (unsigned long)newidt;
-
-	load_idt(&curidt);
-}
-
-
 static void set_gdt(void *newgdt, __u16 limit)
 {
 	struct desc_ptr curgdt;
@@ -245,7 +233,7 @@ void machine_kexec(struct kimage *image)
 	 * If you want to load them you must set up your own idt & gdt.
 	 */
 	set_gdt(phys_to_virt(0), 0);
-	set_idt(phys_to_virt(0), 0);
+	idt_invalidate(phys_to_virt(0));
 
 	/* now call it */
 	image->start = relocate_kernel_ptr((unsigned long)image->head,
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -38,8 +38,6 @@
 void (*pm_power_off)(void);
 EXPORT_SYMBOL(pm_power_off);
 
-static const struct desc_ptr no_idt = {};
-
 /*
  * This is set if we need to go through the 'emergency' path.
  * When machine_emergency_restart() is called, we may be on
@@ -638,7 +636,7 @@ static void native_machine_emergency_res
 			break;
 
 		case BOOT_TRIPLE:
-			load_idt(&no_idt);
+			idt_invalidate(NULL);
 			__asm__ __volatile__("int3");
 
 			/* We're probably dead after this, but... */

[toc] | [prev] | [next] | [standalone]

#1719967 — [patch 06/41] x86/traps: Simplify pagefault tracing logic

Make use of the new irqvector tracing static key and remove the duplicated
trace_do_pagefault() implementation.

If irq vector tracing is disabled, then the overhead of this is a single
NOP5, which is a reasonable tradeoff to avoid duplicated code and the
unholy macro mess.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/entry/entry_32.S    |    8 -------
 arch/x86/entry/entry_64.S    |   13 -----------
 arch/x86/include/asm/traps.h |   10 ---------
 arch/x86/kernel/kvm.c        |    2 -
 arch/x86/mm/fault.c          |   47 ++++++++++---------------------------------
 5 files changed, 15 insertions(+), 65 deletions(-)

--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -891,14 +891,6 @@ BUILD_INTERRUPT3(hyperv_callback_vector,
 
 #endif /* CONFIG_HYPERV */
 
-#ifdef CONFIG_TRACING
-ENTRY(trace_page_fault)
-	ASM_CLAC
-	pushl	$trace_do_page_fault
-	jmp	common_exception
-END(trace_page_fault)
-#endif
-
 ENTRY(page_fault)
 	ASM_CLAC
 	pushl	$do_page_fault
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -829,17 +829,6 @@ ENTRY(\sym)
 END(\sym)
 .endm
 
-#ifdef CONFIG_TRACING
-.macro trace_idtentry sym do_sym has_error_code:req
-idtentry trace(\sym) trace(\do_sym) has_error_code=\has_error_code
-idtentry \sym \do_sym has_error_code=\has_error_code
-.endm
-#else
-.macro trace_idtentry sym do_sym has_error_code:req
-idtentry \sym \do_sym has_error_code=\has_error_code
-.endm
-#endif
-
 idtentry divide_error			do_divide_error			has_error_code=0
 idtentry overflow			do_overflow			has_error_code=0
 idtentry bounds				do_bounds			has_error_code=0
@@ -1004,7 +993,7 @@ idtentry xen_stack_segment	do_stack_segm
 #endif
 
 idtentry general_protection	do_general_protection	has_error_code=1
-trace_idtentry page_fault	do_page_fault		has_error_code=1
+idtentry page_fault		do_page_fault		has_error_code=1
 
 #ifdef CONFIG_KVM_GUEST
 idtentry async_page_fault	do_async_page_fault	has_error_code=1
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -39,7 +39,6 @@ asmlinkage void machine_check(void);
 asmlinkage void simd_coprocessor_error(void);
 
 #ifdef CONFIG_TRACING
-asmlinkage void trace_page_fault(void);
 #define trace_stack_segment stack_segment
 #define trace_divide_error divide_error
 #define trace_bounds bounds
@@ -54,6 +53,7 @@ asmlinkage void trace_page_fault(void);
 #define trace_alignment_check alignment_check
 #define trace_simd_coprocessor_error simd_coprocessor_error
 #define trace_async_page_fault async_page_fault
+#define trace_page_fault page_fault
 #endif
 
 dotraplinkage void do_divide_error(struct pt_regs *, long);
@@ -74,14 +74,6 @@ asmlinkage struct pt_regs *sync_regs(str
 #endif
 dotraplinkage void do_general_protection(struct pt_regs *, long);
 dotraplinkage void do_page_fault(struct pt_regs *, unsigned long);
-#ifdef CONFIG_TRACING
-dotraplinkage void trace_do_page_fault(struct pt_regs *, unsigned long);
-#else
-static inline void trace_do_page_fault(struct pt_regs *regs, unsigned long error)
-{
-	do_page_fault(regs, error);
-}
-#endif
 dotraplinkage void do_spurious_interrupt_bug(struct pt_regs *, long);
 dotraplinkage void do_coprocessor_error(struct pt_regs *, long);
 dotraplinkage void do_alignment_check(struct pt_regs *, long);
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -263,7 +263,7 @@ do_async_page_fault(struct pt_regs *regs
 
 	switch (kvm_read_and_reset_pf_reason()) {
 	default:
-		trace_do_page_fault(regs, error_code);
+		do_page_fault(regs, error_code);
 		break;
 	case KVM_PV_REASON_PAGE_NOT_PRESENT:
 		/* page is swapped out by the host. */
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1254,10 +1254,6 @@ static inline bool smap_violation(int er
  * This routine handles page faults.  It determines the address,
  * and the problem, and then passes it off to one of the appropriate
  * routines.
- *
- * This function must have noinline because both callers
- * {,trace_}do_page_fault() have notrace on. Having this an actual function
- * guarantees there's a function trace entry.
  */
 static noinline void
 __do_page_fault(struct pt_regs *regs, unsigned long error_code,
@@ -1490,27 +1486,6 @@ static noinline void
 }
 NOKPROBE_SYMBOL(__do_page_fault);
 
-dotraplinkage void notrace
-do_page_fault(struct pt_regs *regs, unsigned long error_code)
-{
-	unsigned long address = read_cr2(); /* Get the faulting address */
-	enum ctx_state prev_state;
-
-	/*
-	 * We must have this function tagged with __kprobes, notrace and call
-	 * read_cr2() before calling anything else. To avoid calling any kind
-	 * of tracing machinery before we've observed the CR2 value.
-	 *
-	 * exception_{enter,exit}() contain all sorts of tracepoints.
-	 */
-
-	prev_state = exception_enter();
-	__do_page_fault(regs, error_code, address);
-	exception_exit(prev_state);
-}
-NOKPROBE_SYMBOL(do_page_fault);
-
-#ifdef CONFIG_TRACING
 static nokprobe_inline void
 trace_page_fault_entries(unsigned long address, struct pt_regs *regs,
 			 unsigned long error_code)
@@ -1522,21 +1497,23 @@ trace_page_fault_entries(unsigned long a
 }
 
 dotraplinkage void notrace
-trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
+do_page_fault(struct pt_regs *regs, unsigned long error_code)
 {
+	unsigned long address = read_cr2(); /* Get the faulting address */
+	enum ctx_state prev_state;
+
 	/*
-	 * The exception_enter and tracepoint processing could
-	 * trigger another page faults (user space callchain
-	 * reading) and destroy the original cr2 value, so read
-	 * the faulting address now.
+	 * We must have this function tagged with __kprobes, notrace and call
+	 * read_cr2() before calling anything else. To avoid calling any kind
+	 * of tracing machinery before we've observed the CR2 value.
+	 *
+	 * exception_{enter,exit}() contain all sorts of tracepoints.
 	 */
-	unsigned long address = read_cr2();
-	enum ctx_state prev_state;
+	if (trace_irqvectors_enabled())
+		trace_page_fault_entries(address, regs, error_code);
 
 	prev_state = exception_enter();
-	trace_page_fault_entries(address, regs, error_code);
 	__do_page_fault(regs, error_code, address);
 	exception_exit(prev_state);
 }
-NOKPROBE_SYMBOL(trace_do_page_fault);
-#endif /* CONFIG_TRACING */
+NOKPROBE_SYMBOL(do_page_fault);

[toc] | [prev] | [next] | [standalone]

#1720150 — Re: [patch 06/41] x86/traps: Simplify pagefault tracing logic

On Fri, Aug 25, 2017 at 3:31 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> Make use of the new irqvector tracing static key and remove the duplicated
> trace_do_pagefault() implementation.
>
> If irq vector tracing is disabled, then the overhead of this is a single
> NOP5, which is a reasonable tradeoff to avoid duplicated code and the
> unholy macro mess.

This is great.  Thanks!

[toc] | [prev] | [next] | [standalone]

#1720178 — Re: [patch 06/41] x86/traps: Simplify pagefault tracing logic

On Fri, 25 Aug 2017 12:31:09 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

>  dotraplinkage void notrace
> -trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
> +do_page_fault(struct pt_regs *regs, unsigned long error_code)
>  {
> +	unsigned long address = read_cr2(); /* Get the faulting address */
> +	enum ctx_state prev_state;
> +
>  	/*
> -	 * The exception_enter and tracepoint processing could
> -	 * trigger another page faults (user space callchain
> -	 * reading) and destroy the original cr2 value, so read
> -	 * the faulting address now.
> +	 * We must have this function tagged with __kprobes, notrace and call
> +	 * read_cr2() before calling anything else. To avoid calling any kind
> +	 * of tracing machinery before we've observed the CR2 value.
> +	 *
> +	 * exception_{enter,exit}() contain all sorts of tracepoints.
>  	 */
> -	unsigned long address = read_cr2();
> -	enum ctx_state prev_state;
> +	if (trace_irqvectors_enabled())
> +		trace_page_fault_entries(address, regs, error_code);
>  
>  	prev_state = exception_enter();
> -	trace_page_fault_entries(address, regs, error_code);

I believe you need to keep the tracing after the "exception_enter()", as
for NO_HZ_FULL, that enables RCU again, and the tracepoint needs RCU
enabled.

-- Steve


>  	__do_page_fault(regs, error_code, address);
>  	exception_exit(prev_state);
>  }
> -NOKPROBE_SYMBOL(trace_do_page_fault);
> -#endif /* CONFIG_TRACING */
> +NOKPROBE_SYMBOL(do_page_fault);
> 

[toc] | [prev] | [next] | [standalone]

#1720183 — Re: [patch 06/41] x86/traps: Simplify pagefault tracing logic

On Fri, 25 Aug 2017, Steven Rostedt wrote:

> On Fri, 25 Aug 2017 12:31:09 +0200
> Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> >  dotraplinkage void notrace
> > -trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
> > +do_page_fault(struct pt_regs *regs, unsigned long error_code)
> >  {
> > +	unsigned long address = read_cr2(); /* Get the faulting address */
> > +	enum ctx_state prev_state;
> > +
> >  	/*
> > -	 * The exception_enter and tracepoint processing could
> > -	 * trigger another page faults (user space callchain
> > -	 * reading) and destroy the original cr2 value, so read
> > -	 * the faulting address now.
> > +	 * We must have this function tagged with __kprobes, notrace and call
> > +	 * read_cr2() before calling anything else. To avoid calling any kind
> > +	 * of tracing machinery before we've observed the CR2 value.
> > +	 *
> > +	 * exception_{enter,exit}() contain all sorts of tracepoints.
> >  	 */
> > -	unsigned long address = read_cr2();
> > -	enum ctx_state prev_state;
> > +	if (trace_irqvectors_enabled())
> > +		trace_page_fault_entries(address, regs, error_code);
> >  
> >  	prev_state = exception_enter();
> > -	trace_page_fault_entries(address, regs, error_code);
> 
> I believe you need to keep the tracing after the "exception_enter()", as
> for NO_HZ_FULL, that enables RCU again, and the tracepoint needs RCU
> enabled.

Indeed. Thanks for spotting it.

[toc] | [prev] | [next] | [standalone]

#1719968 — [patch 07/41] x86/apic: Remove the duplicated tracing version of local_timer_interrupt

The two NOP5 are noise in the rest of the work which is done by the timer
interrupt and modern CPUs are pretty good in optimizing nops.

Get rid of the interrupt handler duplication and move the tracepoints into
the regular handler.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/hw_irq.h |    2 +-
 arch/x86/kernel/apic/apic.c   |   19 -------------------
 2 files changed, 1 insertion(+), 20 deletions(-)

--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -48,7 +48,6 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_apic_timer_interrupt(void);
 extern void trace_x86_platform_ipi(void);
 extern void trace_error_interrupt(void);
 extern void trace_irq_work_interrupt(void);
@@ -59,6 +58,7 @@ extern void trace_threshold_interrupt(vo
 extern void trace_deferred_error_interrupt(void);
 extern void trace_call_function_interrupt(void);
 extern void trace_call_function_single_interrupt(void);
+#define trace_apic_timer_interrupt	apic_timer_interrupt
 #define trace_irq_move_cleanup_interrupt  irq_move_cleanup_interrupt
 #define trace_reboot_interrupt  reboot_interrupt
 #define trace_kvm_posted_intr_ipi kvm_posted_intr_ipi
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1040,25 +1040,6 @@ static void local_apic_timer_interrupt(v
 	 * interrupt lock, which is the WrongThing (tm) to do.
 	 */
 	entering_ack_irq();
-	local_apic_timer_interrupt();
-	exiting_irq();
-
-	set_irq_regs(old_regs);
-}
-
-__visible void __irq_entry smp_trace_apic_timer_interrupt(struct pt_regs *regs)
-{
-	struct pt_regs *old_regs = set_irq_regs(regs);
-
-	/*
-	 * NOTE! We'd better ACK the irq immediately,
-	 * because timer handling can be slow.
-	 *
-	 * update_process_times() expects us to have done irq_enter().
-	 * Besides, if we don't timer interrupts ignore the global
-	 * interrupt lock, which is the WrongThing (tm) to do.
-	 */
-	entering_ack_irq();
 	trace_local_timer_entry(LOCAL_TIMER_VECTOR);
 	local_apic_timer_interrupt();
 	trace_local_timer_exit(LOCAL_TIMER_VECTOR);

[toc] | [prev] | [next] | [standalone]

#1720189 — Re: [patch 07/41] x86/apic: Remove the duplicated tracing version of local_timer_interrupt

On Fri, 25 Aug 2017 12:31:10 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> The two NOP5 are noise in the rest of the work which is done by the timer
> interrupt and modern CPUs are pretty good in optimizing nops.
> 
> Get rid of the interrupt handler duplication and move the tracepoints into
> the regular handler.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

[toc] | [prev] | [next] | [standalone]

#1719969 — [patch 14/41] x86/smp: Use static key for reschedule interrupt tracing

It's worth to avoid the extra irq_enter()/irq_exit() pair in the case that
the reschedule interrupt tracepoints are disabled.

Use the static key which indicates that exception tracing is enabled. For
now this key is global. It will be optimized in a later step.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/hw_irq.h |    2 +-
 arch/x86/kernel/smp.c         |   40 ++++++++++++++++------------------------
 2 files changed, 17 insertions(+), 25 deletions(-)

--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -48,7 +48,7 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_reschedule_interrupt(void);
+#define trace_reschedule_interrupt	reschedule_interrupt
 #define trace_call_function_interrupt	call_function_interrupt
 #define trace_call_function_single_interrupt	call_function_single_interrupt
 #define trace_thermal_interrupt	thermal_interrupt
--- a/arch/x86/kernel/smp.c
+++ b/arch/x86/kernel/smp.c
@@ -254,37 +254,29 @@ static void native_stop_other_cpus(int w
 }
 
 /*
- * Reschedule call back.
+ * Reschedule call back. KVM uses this interrupt to force a cpu out of
+ * guest mode
  */
-static inline void __smp_reschedule_interrupt(void)
-{
-	inc_irq_stat(irq_resched_count);
-	scheduler_ipi();
-}
-
 __visible void __irq_entry smp_reschedule_interrupt(struct pt_regs *regs)
 {
 	ack_APIC_irq();
-	__smp_reschedule_interrupt();
-	/*
-	 * KVM uses this interrupt to force a cpu out of guest mode
-	 */
-}
 
-__visible void __irq_entry smp_trace_reschedule_interrupt(struct pt_regs *regs)
-{
-	/*
-	 * Need to call irq_enter() before calling the trace point.
-	 * __smp_reschedule_interrupt() calls irq_enter/exit() too (in
-	 * scheduler_ipi(). This is OK, since those functions are allowed
-	 * to nest.
-	 */
-	ipi_entering_ack_irq();
-	trace_reschedule_entry(RESCHEDULE_VECTOR);
+	if (trace_irqvectors_enabled()) {
+		/*
+		 * scheduler_ipi() might call irq_enter() as well, but
+		 * nested calls are fine.
+		 */
+		irq_enter();
+		trace_reschedule_entry(RESCHEDULE_VECTOR);
+	}
+
 	inc_irq_stat(irq_resched_count);
 	scheduler_ipi();
-	trace_reschedule_exit(RESCHEDULE_VECTOR);
-	exiting_irq();
+
+	if (trace_irqvectors_enabled()) {
+		trace_reschedule_exit(RESCHEDULE_VECTOR);
+		irq_exit();
+	}
 }
 
 __visible void __irq_entry smp_call_function_interrupt(struct pt_regs *regs)

[toc] | [prev] | [next] | [standalone]

#1720379 — Re: [patch 14/41] x86/smp: Use static key for reschedule interrupt tracing

On Fri, 25 Aug 2017 12:31:17 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:


> -__visible void __irq_entry smp_trace_reschedule_interrupt(struct pt_regs *regs)
> -{
> -	/*
> -	 * Need to call irq_enter() before calling the trace point.
> -	 * __smp_reschedule_interrupt() calls irq_enter/exit() too (in
> -	 * scheduler_ipi(). This is OK, since those functions are allowed
> -	 * to nest.
> -	 */
> -	ipi_entering_ack_irq();
> -	trace_reschedule_entry(RESCHEDULE_VECTOR);
> +	if (trace_irqvectors_enabled()) {
> +		/*
> +		 * scheduler_ipi() might call irq_enter() as well, but
> +		 * nested calls are fine.
> +		 */
> +		irq_enter();
> +		trace_reschedule_entry(RESCHEDULE_VECTOR);
> +	}
> +
>  	inc_irq_stat(irq_resched_count);
>  	scheduler_ipi();
> -	trace_reschedule_exit(RESCHEDULE_VECTOR);
> -	exiting_irq();
> +
> +	if (trace_irqvectors_enabled()) {
> +		trace_reschedule_exit(RESCHEDULE_VECTOR);
> +		irq_exit();

This is racy. If the static key gets changed on one and not the other,
we lose the irq_enter/irq_exit match.

-- Steve


> +	}
>  }
>  
>  __visible void __irq_entry smp_call_function_interrupt(struct pt_regs *regs)
> 

[toc] | [prev] | [next] | [standalone]

#1720386 — Re: [patch 14/41] x86/smp: Use static key for reschedule interrupt tracing

On Fri, 25 Aug 2017, Steven Rostedt wrote:
> On Fri, 25 Aug 2017 12:31:17 +0200
> Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> 
> > -__visible void __irq_entry smp_trace_reschedule_interrupt(struct pt_regs *regs)
> > -{
> > -	/*
> > -	 * Need to call irq_enter() before calling the trace point.
> > -	 * __smp_reschedule_interrupt() calls irq_enter/exit() too (in
> > -	 * scheduler_ipi(). This is OK, since those functions are allowed
> > -	 * to nest.
> > -	 */
> > -	ipi_entering_ack_irq();
> > -	trace_reschedule_entry(RESCHEDULE_VECTOR);
> > +	if (trace_irqvectors_enabled()) {
> > +		/*
> > +		 * scheduler_ipi() might call irq_enter() as well, but
> > +		 * nested calls are fine.
> > +		 */
> > +		irq_enter();
> > +		trace_reschedule_entry(RESCHEDULE_VECTOR);
> > +	}
> > +
> >  	inc_irq_stat(irq_resched_count);
> >  	scheduler_ipi();
> > -	trace_reschedule_exit(RESCHEDULE_VECTOR);
> > -	exiting_irq();
> > +
> > +	if (trace_irqvectors_enabled()) {
> > +		trace_reschedule_exit(RESCHEDULE_VECTOR);
> > +		irq_exit();
> 
> This is racy. If the static key gets changed on one and not the other,
> we lose the irq_enter/irq_exit match.

Indeed. Good point. Fixed up version below.

Thanks,

	tglx
8<---------------------
--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -48,7 +48,7 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_reschedule_interrupt(void);
+#define trace_reschedule_interrupt	reschedule_interrupt
 #define trace_call_function_interrupt	call_function_interrupt
 #define trace_call_function_single_interrupt	call_function_single_interrupt
 #define trace_thermal_interrupt	thermal_interrupt
--- a/arch/x86/kernel/smp.c
+++ b/arch/x86/kernel/smp.c
@@ -254,37 +254,27 @@ static void native_stop_other_cpus(int w
 }
 
 /*
- * Reschedule call back.
+ * Reschedule call back. KVM uses this interrupt to force a cpu out of
+ * guest mode
  */
-static inline void __smp_reschedule_interrupt(void)
-{
-	inc_irq_stat(irq_resched_count);
-	scheduler_ipi();
-}
-
 __visible void __irq_entry smp_reschedule_interrupt(struct pt_regs *regs)
 {
 	ack_APIC_irq();
-	__smp_reschedule_interrupt();
-	/*
-	 * KVM uses this interrupt to force a cpu out of guest mode
-	 */
-}
-
-__visible void __irq_entry smp_trace_reschedule_interrupt(struct pt_regs *regs)
-{
-	/*
-	 * Need to call irq_enter() before calling the trace point.
-	 * __smp_reschedule_interrupt() calls irq_enter/exit() too (in
-	 * scheduler_ipi(). This is OK, since those functions are allowed
-	 * to nest.
-	 */
-	ipi_entering_ack_irq();
-	trace_reschedule_entry(RESCHEDULE_VECTOR);
 	inc_irq_stat(irq_resched_count);
+
+	if (trace_irqvectors_enabled()) {
+		/*
+		 * scheduler_ipi() might call irq_enter() as well, but
+		 * nested calls are fine.
+		 */
+		irq_enter();
+		trace_reschedule_entry(RESCHEDULE_VECTOR);
+		scheduler_ipi();
+		trace_reschedule_exit(RESCHEDULE_VECTOR);
+		irq_exit();
+		return;
+	}
 	scheduler_ipi();
-	trace_reschedule_exit(RESCHEDULE_VECTOR);
-	exiting_irq();
 }
 
 __visible void __irq_entry smp_call_function_interrupt(struct pt_regs *regs)

[toc] | [prev] | [next] | [standalone]

#1720417 — Re: [patch 14/41] x86/smp: Use static key for reschedule interrupt tracing

On Fri, 25 Aug 2017 22:07:33 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:

> Fixed up version below.
> 

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

> Thanks,
> 
> 	tglx
> 8<---------------------
> --- a/arch/x86/include/asm/hw_irq.h
> +++ b/arch/x86/include/asm/hw_irq.h
> @@ -48,7 +48,7 @@ extern asmlinkage void call_function_sin
>  
>  #ifdef CONFIG_TRACING
>  /* Interrupt handlers registered during init_IRQ */
> -extern void trace_reschedule_interrupt(void);
> +#define trace_reschedule_interrupt	reschedule_interrupt
>  #define trace_call_function_interrupt	call_function_interrupt
>  #define trace_call_function_single_interrupt	call_function_single_interrupt
>  #define trace_thermal_interrupt	thermal_interrupt
> --- a/arch/x86/kernel/smp.c
> +++ b/arch/x86/kernel/smp.c
> @@ -254,37 +254,27 @@ static void native_stop_other_cpus(int w
>  }
>  
>  /*
> - * Reschedule call back.
> + * Reschedule call back. KVM uses this interrupt to force a cpu out of
> + * guest mode
>   */
> -static inline void __smp_reschedule_interrupt(void)
> -{
> -	inc_irq_stat(irq_resched_count);
> -	scheduler_ipi();
> -}
> -
>  __visible void __irq_entry smp_reschedule_interrupt(struct pt_regs *regs)
>  {
>  	ack_APIC_irq();
> -	__smp_reschedule_interrupt();
> -	/*
> -	 * KVM uses this interrupt to force a cpu out of guest mode
> -	 */
> -}
> -
> -__visible void __irq_entry smp_trace_reschedule_interrupt(struct pt_regs *regs)
> -{
> -	/*
> -	 * Need to call irq_enter() before calling the trace point.
> -	 * __smp_reschedule_interrupt() calls irq_enter/exit() too (in
> -	 * scheduler_ipi(). This is OK, since those functions are allowed
> -	 * to nest.
> -	 */
> -	ipi_entering_ack_irq();
> -	trace_reschedule_entry(RESCHEDULE_VECTOR);
>  	inc_irq_stat(irq_resched_count);
> +
> +	if (trace_irqvectors_enabled()) {
> +		/*
> +		 * scheduler_ipi() might call irq_enter() as well, but
> +		 * nested calls are fine.
> +		 */
> +		irq_enter();
> +		trace_reschedule_entry(RESCHEDULE_VECTOR);
> +		scheduler_ipi();
> +		trace_reschedule_exit(RESCHEDULE_VECTOR);
> +		irq_exit();
> +		return;
> +	}
>  	scheduler_ipi();
> -	trace_reschedule_exit(RESCHEDULE_VECTOR);
> -	exiting_irq();
>  }
>  
>  __visible void __irq_entry smp_call_function_interrupt(struct pt_regs *regs)
> 

[toc] | [prev] | [next] | [standalone]

#1719970 — [patch 05/41] x86/tracing: Introduce a static key for exception tracing

Switching the IDT just for avoiding tracepoints creates a complete
impenetrable macro/inline/ifdef mess.

There is no point in avoiding tracepoints for most of the traps/exceptions.
For the more expensive tracepoints, like pagefaults, this can be handled with
an explicit static key.

Preparatory patch to remove the tracing idt.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/trace/common.h      |   15 +++++++++++++++
 arch/x86/include/asm/trace/exceptions.h  |    4 +---
 arch/x86/include/asm/trace/irq_vectors.h |    4 +---
 arch/x86/kernel/tracepoint.c             |    9 ++++++++-
 4 files changed, 25 insertions(+), 7 deletions(-)

--- /dev/null
+++ b/arch/x86/include/asm/trace/common.h
@@ -0,0 +1,15 @@
+#ifndef _ASM_TRACE_COMMON_H
+#define _ASM_TRACE_COMMON_H
+
+extern int trace_irq_vector_regfunc(void);
+extern void trace_irq_vector_unregfunc(void);
+
+#ifdef CONFIG_TRACING
+DECLARE_STATIC_KEY_FALSE(trace_irqvectors_key);
+#define trace_irqvectors_enabled()			\
+	static_branch_unlikely(&trace_irqvectors_key)
+#else
+static inline bool trace_irqvectors_enabled(void) { return false; }
+#endif
+
+#endif
--- a/arch/x86/include/asm/trace/exceptions.h
+++ b/arch/x86/include/asm/trace/exceptions.h
@@ -5,9 +5,7 @@
 #define _TRACE_PAGE_FAULT_H
 
 #include <linux/tracepoint.h>
-
-extern int trace_irq_vector_regfunc(void);
-extern void trace_irq_vector_unregfunc(void);
+#include <asm/trace/common.h>
 
 DECLARE_EVENT_CLASS(x86_exceptions,
 
--- a/arch/x86/include/asm/trace/irq_vectors.h
+++ b/arch/x86/include/asm/trace/irq_vectors.h
@@ -5,9 +5,7 @@
 #define _TRACE_IRQ_VECTORS_H
 
 #include <linux/tracepoint.h>
-
-extern int trace_irq_vector_regfunc(void);
-extern void trace_irq_vector_unregfunc(void);
+#include <asm/trace/common.h>
 
 DECLARE_EVENT_CLASS(x86_irq_vector,
 
--- a/arch/x86/kernel/tracepoint.c
+++ b/arch/x86/kernel/tracepoint.c
@@ -4,9 +4,11 @@
  * Copyright (C) 2013 Seiji Aguchi <seiji.aguchi@hds.com>
  *
  */
+#include <linux/jump_label.h>
+#include <linux/atomic.h>
+
 #include <asm/hw_irq.h>
 #include <asm/desc.h>
-#include <linux/atomic.h>
 
 atomic_t trace_idt_ctr = ATOMIC_INIT(0);
 struct desc_ptr trace_idt_descr = { NR_VECTORS * 16 - 1,
@@ -15,6 +17,7 @@ struct desc_ptr trace_idt_descr = { NR_V
 /* No need to be aligned, but done to keep all IDTs defined the same way. */
 gate_desc trace_idt_table[NR_VECTORS] __page_aligned_bss;
 
+DEFINE_STATIC_KEY_FALSE(trace_irqvectors_key);
 static int trace_irq_vector_refcount;
 static DEFINE_MUTEX(irq_vector_mutex);
 
@@ -36,6 +39,8 @@ static void switch_idt(void *arg)
 
 int trace_irq_vector_regfunc(void)
 {
+	static_branch_inc(&trace_irqvectors_key);
+
 	mutex_lock(&irq_vector_mutex);
 	if (!trace_irq_vector_refcount) {
 		set_trace_idt_ctr(1);
@@ -49,6 +54,8 @@ int trace_irq_vector_regfunc(void)
 
 void trace_irq_vector_unregfunc(void)
 {
+	static_branch_dec(&trace_irqvectors_key);
+
 	mutex_lock(&irq_vector_mutex);
 	trace_irq_vector_refcount--;
 	if (!trace_irq_vector_refcount) {

[toc] | [prev] | [next] | [standalone]

#1720192 — Re: [patch 05/41] x86/tracing: Introduce a static key for exception tracing

On Fri, 25 Aug 2017 12:31:08 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> Switching the IDT just for avoiding tracepoints creates a complete
> impenetrable macro/inline/ifdef mess.
> 
> There is no point in avoiding tracepoints for most of the traps/exceptions.
> For the more expensive tracepoints, like pagefaults, this can be handled with
> an explicit static key.
> 
> Preparatory patch to remove the tracing idt.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/include/asm/trace/common.h      |   15 +++++++++++++++
>  arch/x86/include/asm/trace/exceptions.h  |    4 +---
>  arch/x86/include/asm/trace/irq_vectors.h |    4 +---
>  arch/x86/kernel/tracepoint.c             |    9 ++++++++-
>  4 files changed, 25 insertions(+), 7 deletions(-)
> 
> --- /dev/null
> +++ b/arch/x86/include/asm/trace/common.h

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

[toc] | [prev] | [next] | [standalone]

#1719972 — [patch 10/41] x86/apic: Remove the duplicated tracing versions of interrupts

The error and the spurious interrupt are really rare events and not at all
so performance sensitive that two NOP5s can not be tolerated when tracing
is disabled.

Remove the nonsense.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/hw_irq.h |    4 +--
 arch/x86/kernel/apic/apic.c   |   43 +++++++++---------------------------------
 2 files changed, 12 insertions(+), 35 deletions(-)

--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -48,15 +48,15 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_error_interrupt(void);
 extern void trace_irq_work_interrupt(void);
-extern void trace_spurious_interrupt(void);
 extern void trace_thermal_interrupt(void);
 extern void trace_reschedule_interrupt(void);
 extern void trace_threshold_interrupt(void);
 extern void trace_deferred_error_interrupt(void);
 extern void trace_call_function_interrupt(void);
 extern void trace_call_function_single_interrupt(void);
+#define trace_error_interrupt error_interrupt
+#define trace_spurious_interrupt spurious_interrupt
 #define trace_x86_platform_ipi	x86_platform_ipi
 #define trace_apic_timer_interrupt	apic_timer_interrupt
 #define trace_irq_move_cleanup_interrupt  irq_move_cleanup_interrupt
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1901,10 +1901,14 @@ void __init register_lapic_address(unsig
 /*
  * This interrupt should _never_ happen with our APIC/SMP architecture
  */
-static void __smp_spurious_interrupt(u8 vector)
+__visible void __irq_entry smp_spurious_interrupt(struct pt_regs *regs)
 {
+	u8 vector = ~regs->orig_ax;
 	u32 v;
 
+	entering_irq();
+	trace_spurious_apic_entry(vector);
+
 	/*
 	 * Check if this really is a spurious interrupt and ACK it
 	 * if it is a vectored one.  Just in case...
@@ -1919,22 +1923,7 @@ static void __smp_spurious_interrupt(u8
 	/* see sw-dev-man vol 3, chapter 7.4.13.5 */
 	pr_info("spurious APIC interrupt through vector %02x on CPU#%d, "
 		"should never happen.\n", vector, smp_processor_id());
-}
 
-__visible void __irq_entry smp_spurious_interrupt(struct pt_regs *regs)
-{
-	entering_irq();
-	__smp_spurious_interrupt(~regs->orig_ax);
-	exiting_irq();
-}
-
-__visible void __irq_entry smp_trace_spurious_interrupt(struct pt_regs *regs)
-{
-	u8 vector = ~regs->orig_ax;
-
-	entering_irq();
-	trace_spurious_apic_entry(vector);
-	__smp_spurious_interrupt(vector);
 	trace_spurious_apic_exit(vector);
 	exiting_irq();
 }
@@ -1942,10 +1931,8 @@ static void __smp_spurious_interrupt(u8
 /*
  * This interrupt should never happen with our APIC/SMP architecture
  */
-static void __smp_error_interrupt(struct pt_regs *regs)
+__visible void __irq_entry smp_error_interrupt(struct pt_regs *regs)
 {
-	u32 v;
-	u32 i = 0;
 	static const char * const error_interrupt_reason[] = {
 		"Send CS error",		/* APIC Error Bit 0 */
 		"Receive CS error",		/* APIC Error Bit 1 */
@@ -1956,6 +1943,10 @@ static void __smp_error_interrupt(struct
 		"Received illegal vector",	/* APIC Error Bit 6 */
 		"Illegal register address",	/* APIC Error Bit 7 */
 	};
+	u32 v, i = 0;
+
+	entering_irq();
+	trace_error_apic_entry(ERROR_APIC_VECTOR);
 
 	/* First tickle the hardware, only then report what went on. -- REW */
 	if (lapic_get_maxlvt() > 3)	/* Due to the Pentium erratum 3AP. */
@@ -1977,20 +1968,6 @@ static void __smp_error_interrupt(struct
 
 	apic_printk(APIC_DEBUG, KERN_CONT "\n");
 
-}
-
-__visible void __irq_entry smp_error_interrupt(struct pt_regs *regs)
-{
-	entering_irq();
-	__smp_error_interrupt(regs);
-	exiting_irq();
-}
-
-__visible void __irq_entry smp_trace_error_interrupt(struct pt_regs *regs)
-{
-	entering_irq();
-	trace_error_apic_entry(ERROR_APIC_VECTOR);
-	__smp_error_interrupt(regs);
 	trace_error_apic_exit(ERROR_APIC_VECTOR);
 	exiting_irq();
 }

[toc] | [prev] | [next] | [standalone]

#1720203 — Re: [patch 10/41] x86/apic: Remove the duplicated tracing versions of interrupts

On Fri, 25 Aug 2017 12:31:13 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> The error and the spurious interrupt are really rare events and not at all
> so performance sensitive that two NOP5s can not be tolerated when tracing
> is disabled.

Just a note. I'm sure if we disassembled it, it may be a little more
work done than just two NOPs, as parameter passing to the tracepoints
sometimes leak out of the static jump block. It's moot on this patch,
but other irqs with fast paths may need to be looked at.

> 
> Remove the nonsense.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/include/asm/hw_irq.h |    4 +--
>  arch/x86/kernel/apic/apic.c   |   43 +++++++++---------------------------------
>  2 files changed, 12 insertions(+), 35 deletions(-)
> 

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

[toc] | [prev] | [next] | [standalone]

#1721129 — Re: [patch 10/41] x86/apic: Remove the duplicated tracing versions of interrupts

On Fri, Aug 25, 2017 at 11:49:47AM -0400, Steven Rostedt wrote:
> On Fri, 25 Aug 2017 12:31:13 +0200
> Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > The error and the spurious interrupt are really rare events and not at all
> > so performance sensitive that two NOP5s can not be tolerated when tracing
> > is disabled.
> 
> Just a note. I'm sure if we disassembled it, it may be a little more
> work done than just two NOPs, as parameter passing to the tracepoints
> sometimes leak out of the static jump block. It's moot on this patch,
> but other irqs with fast paths may need to be looked at.

Is that something we can fix with the trace macros?

They have a general shape of:

#define trace_foo(args...)
	if (static_branch_unlikely(&foo_enabled)) {
		__trace_foo(args...);
	}

Right? And I suppose I see why the compiler would want to sometimes lift
stuff out of the branch block, but we'd really like it not to do that.
Would putting a barrier() in front of __trace_foo() help?

[toc] | [prev] | [next] | [standalone]

#1719974 — [patch 11/41] x86/irqwork: Get rid of duplicated tracing interrupt code

Two NOP5 are a reasonable tradeoff to avoid duplicated code and the
requirement to switch the IDT.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/hw_irq.h |    2 +-
 arch/x86/kernel/irq_work.c    |   16 ++--------------
 2 files changed, 3 insertions(+), 15 deletions(-)

--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -48,13 +48,13 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_irq_work_interrupt(void);
 extern void trace_thermal_interrupt(void);
 extern void trace_reschedule_interrupt(void);
 extern void trace_threshold_interrupt(void);
 extern void trace_deferred_error_interrupt(void);
 extern void trace_call_function_interrupt(void);
 extern void trace_call_function_single_interrupt(void);
+#define trace_irq_work_interrupt irq_work_interrupt
 #define trace_error_interrupt error_interrupt
 #define trace_spurious_interrupt spurious_interrupt
 #define trace_x86_platform_ipi	x86_platform_ipi
--- a/arch/x86/kernel/irq_work.c
+++ b/arch/x86/kernel/irq_work.c
@@ -11,24 +11,12 @@
 #include <asm/trace/irq_vectors.h>
 #include <linux/interrupt.h>
 
-static inline void __smp_irq_work_interrupt(void)
-{
-	inc_irq_stat(apic_irq_work_irqs);
-	irq_work_run();
-}
-
 __visible void __irq_entry smp_irq_work_interrupt(struct pt_regs *regs)
 {
 	ipi_entering_ack_irq();
-	__smp_irq_work_interrupt();
-	exiting_irq();
-}
-
-__visible void __irq_entry smp_trace_irq_work_interrupt(struct pt_regs *regs)
-{
-	ipi_entering_ack_irq();
+	inc_irq_stat(apic_irq_work_irqs);
 	trace_irq_work_entry(IRQ_WORK_VECTOR);
-	__smp_irq_work_interrupt();
+	irq_work_run();
 	trace_irq_work_exit(IRQ_WORK_VECTOR);
 	exiting_irq();
 }

[toc] | [prev] | [next] | [standalone]

#1720207 — Re: [patch 11/41] x86/irqwork: Get rid of duplicated tracing interrupt code

On Fri, 25 Aug 2017 12:31:14 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:


> --- a/arch/x86/kernel/irq_work.c
> +++ b/arch/x86/kernel/irq_work.c
> @@ -11,24 +11,12 @@
>  #include <asm/trace/irq_vectors.h>
>  #include <linux/interrupt.h>
>  
> -static inline void __smp_irq_work_interrupt(void)
> -{
> -	inc_irq_stat(apic_irq_work_irqs);
> -	irq_work_run();
> -}
> -
>  __visible void __irq_entry smp_irq_work_interrupt(struct pt_regs *regs)
>  {
>  	ipi_entering_ack_irq();
> -	__smp_irq_work_interrupt();
> -	exiting_irq();
> -}
> -
> -__visible void __irq_entry smp_trace_irq_work_interrupt(struct pt_regs *regs)
> -{
> -	ipi_entering_ack_irq();
> +	inc_irq_stat(apic_irq_work_irqs);
>  	trace_irq_work_entry(IRQ_WORK_VECTOR);
> -	__smp_irq_work_interrupt();

This changes from what we use to have:

Original:

	ipi_entering_ack_irq();
	trace_irq_work_entry(IRQ_WORK_VECTOR);
	inc_irq_stat(apic_irq_work_irqs);
	irq_work_run();

Patched:

	ipi_entering_ack_irq();
	inc_irq_stat(apic_irq_work_irqs);
	trace_irq_work_entry(IRQ_WORK_VECTOR);
	irq_work_run();

Is there a reason for the swap? It's not mentioned in the change log.

Also, tracepoints can issue new irq works. I don't think that would be
a problem here, as interrupts are disabled in this path. Just wanted to
point it out.

-- Steve


> +	irq_work_run();
>  	trace_irq_work_exit(IRQ_WORK_VECTOR);
>  	exiting_irq();
>  }
> 

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web