[PATCH v3 RESEND] x86/tracing: introduce enter/exit tracepoint pairs for page faults

[PATCH v3 RESEND] x86/tracing: introduce enter/exit tracepoint pairs for page faults

[Junxuan Liao]

Merge page_fault_{user,kernel}, rename it page_fault_enter, and add
page_fault_exit. This pair is useful for measuring page fault handling
latencies.

Add a new field to the merged tracepoints to indicate whether the page
fault happened in userspace. We no longer need the static key associated,
since it was used just to avoid checking user_mode when the tracepoints
were disabled.

Signed-off-by: Junxuan Liao <ljx@cs.wisc.edu>
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Link: https://lore.kernel.org/9e2ac1e3-d07d-4f17-970e-6b7a5248a5bb@cs.wisc.edu
---
v1: https://lore.kernel.org/linux-trace-kernel/e7d4cd81-c0a5-446c-95d2-6142d660c15b@cs.wisc.edu/
v1 -> v2:
Merge the user and kerenl tracepoints. Remove the static keys.
v2 -> v3:
Added back trace.o. Sorry that I accidentally deleted that in v2.

  arch/x86/include/asm/trace/common.h      | 12 ------------
  arch/x86/include/asm/trace/exceptions.h  | 15 +++++++--------
  arch/x86/include/asm/trace/irq_vectors.h |  1 -
  arch/x86/kernel/Makefile                 |  1 -
  arch/x86/kernel/tracepoint.c             | 21 ---------------------
  arch/x86/mm/fault.c                      | 16 ++--------------
  6 files changed, 9 insertions(+), 57 deletions(-)
  delete mode 100644 arch/x86/include/asm/trace/common.h
  delete mode 100644 arch/x86/kernel/tracepoint.c

diff --git a/arch/x86/include/asm/trace/common.h b/arch/x86/include/asm/trace/common.h
deleted file mode 100644
index f0f9bcdb74d9..000000000000
--- a/arch/x86/include/asm/trace/common.h
+++ /dev/null
@@ -1,12 +0,0 @@
-#ifndef _ASM_TRACE_COMMON_H
-#define _ASM_TRACE_COMMON_H
-
-#ifdef CONFIG_TRACING
-DECLARE_STATIC_KEY_FALSE(trace_pagefault_key);
-#define trace_pagefault_enabled()			\
-	static_branch_unlikely(&trace_pagefault_key)
-#else
-static inline bool trace_pagefault_enabled(void) { return false; }
-#endif
-
-#endif
diff --git a/arch/x86/include/asm/trace/exceptions.h b/arch/x86/include/asm/trace/exceptions.h
index 6b1e87194809..f98c9024cbe3 100644
--- a/arch/x86/include/asm/trace/exceptions.h
+++ b/arch/x86/include/asm/trace/exceptions.h
@@ -6,10 +6,6 @@
  #define _TRACE_PAGE_FAULT_H
  
  #include <linux/tracepoint.h>
-#include <asm/trace/common.h>
-
-extern int trace_pagefault_reg(void);
-extern void trace_pagefault_unreg(void);
  
  DECLARE_EVENT_CLASS(x86_exceptions,
  
@@ -21,17 +17,20 @@ DECLARE_EVENT_CLASS(x86_exceptions,
  	TP_STRUCT__entry(
  		__field(		unsigned long, address	)
  		__field(		unsigned long, ip	)
+		__field(		bool         , user_mode)
  		__field(		unsigned long, error_code )
  	),
  
  	TP_fast_assign(
  		__entry->address = address;
  		__entry->ip = regs->ip;
+		__entry->user_mode = user_mode(regs);
  		__entry->error_code = error_code;
  	),
  
-	TP_printk("address=%ps ip=%ps error_code=0x%lx",
+	TP_printk("address=%ps ip=%ps %s error_code=0x%lx",
  		  (void *)__entry->address, (void *)__entry->ip,
+		  __entry->user_mode ? "user" : "kernel",
  		  __entry->error_code) );
  
  #define DEFINE_PAGE_FAULT_EVENT(name)				\
@@ -39,10 +38,10 @@ DEFINE_EVENT_FN(x86_exceptions, name,				\
  	TP_PROTO(unsigned long address,	struct pt_regs *regs,	\
  		 unsigned long error_code),			\
  	TP_ARGS(address, regs, error_code),			\
-	trace_pagefault_reg, trace_pagefault_unreg);
+	NULL, NULL)
  
-DEFINE_PAGE_FAULT_EVENT(page_fault_user);
-DEFINE_PAGE_FAULT_EVENT(page_fault_kernel);
+DEFINE_PAGE_FAULT_EVENT(page_fault_enter);
+DEFINE_PAGE_FAULT_EVENT(page_fault_exit);
  
  #undef TRACE_INCLUDE_PATH
  #undef TRACE_INCLUDE_FILE
diff --git a/arch/x86/include/asm/trace/irq_vectors.h b/arch/x86/include/asm/trace/irq_vectors.h
index 88e7f0f3bf62..7408bebdfde0 100644
--- a/arch/x86/include/asm/trace/irq_vectors.h
+++ b/arch/x86/include/asm/trace/irq_vectors.h
@@ -6,7 +6,6 @@
  #define _TRACE_IRQ_VECTORS_H
  
  #include <linux/tracepoint.h>
-#include <asm/trace/common.h>
  
  #ifdef CONFIG_X86_LOCAL_APIC
  
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index b43eb7e384eb..e8e33ec684ba 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -139,7 +139,6 @@ obj-$(CONFIG_OF)			+= devicetree.o
  obj-$(CONFIG_UPROBES)			+= uprobes.o
  
  obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o
-obj-$(CONFIG_TRACING)			+= tracepoint.o
  obj-$(CONFIG_SCHED_MC_PRIO)		+= itmt.o
  obj-$(CONFIG_X86_UMIP)			+= umip.o
  
diff --git a/arch/x86/kernel/tracepoint.c b/arch/x86/kernel/tracepoint.c
deleted file mode 100644
index 03ae1caaa878..000000000000
--- a/arch/x86/kernel/tracepoint.c
+++ /dev/null
@@ -1,21 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 2013 Seiji Aguchi <seiji.aguchi@hds.com>
- */
-#include <linux/jump_label.h>
-#include <linux/atomic.h>
-
-#include <asm/trace/exceptions.h>
-
-DEFINE_STATIC_KEY_FALSE(trace_pagefault_key);
-
-int trace_pagefault_reg(void)
-{
-	static_branch_inc(&trace_pagefault_key);
-	return 0;
-}
-
-void trace_pagefault_unreg(void)
-{
-	static_branch_dec(&trace_pagefault_key);
-}
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 296d294142c8..eda312707fde 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1451,24 +1451,10 @@ void do_user_addr_fault(struct pt_regs *regs,
  }
  NOKPROBE_SYMBOL(do_user_addr_fault);
  
-static __always_inline void
-trace_page_fault_entries(struct pt_regs *regs, unsigned long error_code,
-			 unsigned long address)
-{
-	if (!trace_pagefault_enabled())
-		return;
-
-	if (user_mode(regs))
-		trace_page_fault_user(address, regs, error_code);
-	else
-		trace_page_fault_kernel(address, regs, error_code);
-}
-
  static __always_inline void
  handle_page_fault(struct pt_regs *regs, unsigned long error_code,
  			      unsigned long address)
  {
-	trace_page_fault_entries(regs, error_code, address);
  
  	if (unlikely(kmmio_fault(regs, address)))
  		return;
@@ -1535,7 +1521,9 @@ DEFINE_IDTENTRY_RAW_ERRORCODE(exc_page_fault)
  	state = irqentry_enter(regs);
  
  	instrumentation_begin();
+	trace_page_fault_enter(address, regs, error_code);
  	handle_page_fault(regs, error_code, address);
+	trace_page_fault_exit(address, regs, error_code);
  	instrumentation_end();
  
  	irqentry_exit(regs, state);
-- 
2.48.1

Re: [PATCH v3 RESEND] x86/tracing: introduce enter/exit tracepoint pairs for page faults

[Nam Cao]

On Sun, Jun 15, 2025 at 07:32:00PM -0500, Junxuan Liao wrote:
> Merge page_fault_{user,kernel}, rename it page_fault_enter, and add
> page_fault_exit. This pair is useful for measuring page fault handling
> latencies.
> 
> Add a new field to the merged tracepoints to indicate whether the page
> fault happened in userspace. We no longer need the static key associated,
> since it was used just to avoid checking user_mode when the tracepoints
> were disabled.

(I think) this breaks lttng which uses the old tracepoint names [1]. But
I'm not sure if we should worry about that.

Do you plan to add this to other architectures?

Perhaps it is better to implement the other approach suggested by Frederic
[2], using trace_user_exit/trace_user_enter? So that more architectures are
covered, and avoid breaking user tools?

(Btw, you would need to rebase this patch. The tracepoint has been moved,
and the static key has been deleted.)

Best regards,
Nam

[1] https://github.com/lttng/lttng-modules/blob/master/include/instrumentation/events/arch/x86/exceptions.h#L88C48-L88C63
[2] https://lore.kernel.org/lkml/Z_mO6_m0bV-Q8NEa@pavilion.home/#t