[RFC 0/5] perf, signal x86: Fix breakpoint events overflow handling

[RFC 0/5] perf, signal x86: Fix breakpoint events overflow handling

[Jiri Olsa]

hi,
while porting Vince's overflow tests I found perf event
breakpoint overflow does not work properly.

I found the x86 RF EFLAG bit not being set when returning
from debug exception after triggering signal handler. Which
is exactly what you get when you set perf breakpoint overflow
SIGIO handler.

Patches 1 and 2 are trying to fix that and make attached
tests pass. But it's very likely I'm breaking something
else by these changes, so please consider those as RFC
patches.. pure suggestions to make my point ;-)

Patch 3 fixes the period handling for breakpoint events.

Patches 4 and 5 provide automated perf tests for testing
all 3 kernel changes.

Also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/linux.git
perf/RF1

thanks for comments,
jirka


Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
---
Jiri Olsa (5):
      signal x86: Propage RF EFLAGS bit throught the signal restore call
      signal x86: Clear RF EFLAGS bit for signal handler
      perf: Fix hw breakpoints overflow period sampling
      perf tests: Test breakpoint overflow signal handler
      perf tests: Test breakpoint overflow signal handler counts

 arch/x86/ia32/ia32_signal.c           |   2 -
 arch/x86/include/asm/sighandling.h    |   4 +-
 arch/x86/kernel/signal.c              |  13 ++++---
 include/linux/perf_event.h            |   2 +
 kernel/events/core.c                  |   2 +-
 kernel/events/hw_breakpoint.c         |   5 +++
 tools/perf/Makefile                   |   2 +
 tools/perf/tests/bp_signal.c          | 187 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/bp_signal_overflow.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/builtin-test.c       |   8 ++++
 tools/perf/tests/tests.h              |   2 +
 11 files changed, 342 insertions(+), 11 deletions(-)
 create mode 100644 tools/perf/tests/bp_signal.c
 create mode 100644 tools/perf/tests/bp_signal_overflow.c

[PATCH 1/5] signal x86: Propage RF EFLAGS bit throught the signal restore call

[Jiri Olsa]

Adding RF EFLAGS bit to be restored on return from signal from
the original register context before the signal was entered.

This will prevent the RF flag to disappear when returning
from exception due to the signal handler being executed.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
---
 arch/x86/ia32/ia32_signal.c        | 2 --
 arch/x86/include/asm/sighandling.h | 4 ++--
 arch/x86/kernel/signal.c           | 6 ------
 3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index a1daf4a..f13c279 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -34,8 +34,6 @@
 #include <asm/sys_ia32.h>
 #include <asm/smap.h>
 
-#define FIX_EFLAGS	__FIX_EFLAGS
-
 int copy_siginfo_to_user32(compat_siginfo_t __user *to, siginfo_t *from)
 {
 	int err = 0;
diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
index beff97f..7a95816 100644
--- a/arch/x86/include/asm/sighandling.h
+++ b/arch/x86/include/asm/sighandling.h
@@ -7,10 +7,10 @@
 
 #include <asm/processor-flags.h>
 
-#define __FIX_EFLAGS	(X86_EFLAGS_AC | X86_EFLAGS_OF | \
+#define FIX_EFLAGS	(X86_EFLAGS_AC | X86_EFLAGS_OF | \
 			 X86_EFLAGS_DF | X86_EFLAGS_TF | X86_EFLAGS_SF | \
 			 X86_EFLAGS_ZF | X86_EFLAGS_AF | X86_EFLAGS_PF | \
-			 X86_EFLAGS_CF)
+			 X86_EFLAGS_CF | X86_EFLAGS_RF)
 
 void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
 
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index d6bf1f3..b6fe116 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -43,12 +43,6 @@
 
 #include <asm/sigframe.h>
 
-#ifdef CONFIG_X86_32
-# define FIX_EFLAGS	(__FIX_EFLAGS | X86_EFLAGS_RF)
-#else
-# define FIX_EFLAGS	__FIX_EFLAGS
-#endif
-
 #define COPY(x)			do {			\
 	get_user_ex(regs->x, &sc->x);			\
 } while (0)
-- 
1.7.11.7

[PATCH 2/5] signal x86: Clear RF EFLAGS bit for signal handler

[Jiri Olsa]

Clearing RF EFLAGS bit for signal handler. The reason is,
that this flag is set by debug exception code to prevent
the recursive exception entry.

Leaving it set for signal handler might prevent debug
exception of the signal handler itself.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
---
 arch/x86/kernel/signal.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index b6fe116..e273571 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -726,6 +726,13 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
 	regs->flags &= ~X86_EFLAGS_DF;
 
 	/*
+	 * Clear RF when entering the signal handler, because
+	 * it might disable possible debug exception from the
+	 * signal handler.
+	 */
+	regs->flags &= ~X86_EFLAGS_RF;
+
+	/*
 	 * Clear TF when entering the signal handler, but
 	 * notify any tracer that was single-stepping it.
 	 * The tracer may want to single-step inside the
-- 
1.7.11.7

[PATCH 3/5] perf: Fix hw breakpoints overflow period sampling

[Jiri Olsa]

The hw breakpoint pmu 'add' function is missing the
period_left update needed for SW events.

The perf HW breakpoint events use SW events framework
for to process the overflow, so it needs to be properly
initialized during PMU 'add' method.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
---
 include/linux/perf_event.h    | 2 ++
 kernel/events/core.c          | 2 +-
 kernel/events/hw_breakpoint.c | 5 +++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index e47ee46..a0ec1fd 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -754,6 +754,7 @@ extern unsigned int perf_output_skip(struct perf_output_handle *handle,
 				     unsigned int len);
 extern int perf_swevent_get_recursion_context(void);
 extern void perf_swevent_put_recursion_context(int rctx);
+extern u64 perf_swevent_set_period(struct perf_event *event);
 extern void perf_event_enable(struct perf_event *event);
 extern void perf_event_disable(struct perf_event *event);
 extern int __perf_event_disable(void *info);
@@ -793,6 +794,7 @@ static inline void perf_event_fork(struct task_struct *tsk)		{ }
 static inline void perf_event_init(void)				{ }
 static inline int  perf_swevent_get_recursion_context(void)		{ return -1; }
 static inline void perf_swevent_put_recursion_context(int rctx)		{ }
+static inline u64 perf_swevent_set_period(struct perf_event *event)	{ return 0; }
 static inline void perf_event_enable(struct perf_event *event)		{ }
 static inline void perf_event_disable(struct perf_event *event)		{ }
 static inline int __perf_event_disable(void *info)			{ return -1; }
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5c75791..de6528b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4957,7 +4957,7 @@ static DEFINE_PER_CPU(struct swevent_htable, swevent_htable);
  * sign as trigger.
  */
 
-static u64 perf_swevent_set_period(struct perf_event *event)
+u64 perf_swevent_set_period(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
 	u64 period = hwc->last_period;
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index a64f8ae..966a241 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -612,6 +612,11 @@ static int hw_breakpoint_add(struct perf_event *bp, int flags)
 	if (!(flags & PERF_EF_START))
 		bp->hw.state = PERF_HES_STOPPED;
 
+	if (is_sampling_event(bp)) {
+		bp->hw.last_period = bp->hw.sample_period;
+		perf_swevent_set_period(bp);
+	}
+
 	return arch_install_hw_breakpoint(bp);
 }
 
-- 
1.7.11.7

[PATCH 4/5] perf tests: Test breakpoint overflow signal handler

[Jiri Olsa]

Adding automated test for breakpoint event signal
handler checking if it's executed properly.

The test is related to the proper handling of the RF
EFLAGS bit on x86_64, but it's generic for all archs.

First we check the signal handler is properly called
and that the following debug exception return to user
space wouldn't trigger recursive breakpoint.

This is related to x86_64 RF EFLAGS bit being managed
in a wrong way.

Second we check that we can set breakpoint in signal
handler, which is not possible on x86_64 if the signal
handler is executed with RF EFLAG set.

This test is inpired by overflow tests done by Vince
Weaver.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
---
 tools/perf/Makefile             |   1 +
 tools/perf/tests/bp_signal.c    | 187 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/builtin-test.c |   4 +
 tools/perf/tests/tests.h        |   1 +
 4 files changed, 193 insertions(+)
 create mode 100644 tools/perf/tests/bp_signal.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index a2108ca..bea66fe 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -497,6 +497,7 @@ LIB_OBJS += $(OUTPUT)tests/evsel-tp-sched.o
 LIB_OBJS += $(OUTPUT)tests/pmu.o
 LIB_OBJS += $(OUTPUT)tests/hists_link.o
 LIB_OBJS += $(OUTPUT)tests/python-use.o
+LIB_OBJS += $(OUTPUT)tests/bp_signal.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c
new file mode 100644
index 0000000..a40b019
--- /dev/null
+++ b/tools/perf/tests/bp_signal.c
@@ -0,0 +1,187 @@
+/*
+ * Inspired by breakpoint overflow test done by
+ * Vince Weaver <vincent.weaver@maine.edu> for perf_event_tests
+ * (git://github.com/deater/perf_event_tests)
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <time.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <linux/compiler.h>
+#include <linux/hw_breakpoint.h>
+
+#include "tests.h"
+#include "debug.h"
+#include "perf.h"
+
+static int fd1;
+static int fd2;
+static int overflows;
+
+__attribute__ ((noinline))
+static int test_function(void)
+{
+	return time(NULL);
+}
+
+static void sig_handler(int signum __maybe_unused,
+			siginfo_t *oh __maybe_unused,
+			void *uc __maybe_unused)
+{
+	overflows++;
+
+	if (overflows > 10) {
+		/*
+		 * This should be executed only once during
+		 * this test, if we are here for the 10th
+		 * time, consider this the recursive issue.
+		 *
+		 * We can get out of here by disable events,
+		 * so no new SIGIO is delivered.
+		 */
+		ioctl(fd1, PERF_EVENT_IOC_DISABLE, 0);
+		ioctl(fd2, PERF_EVENT_IOC_DISABLE, 0);
+	}
+}
+
+static int bp_event(void *fn, int setup_signal)
+{
+	struct perf_event_attr pe;
+	int fd;
+
+	memset(&pe, 0, sizeof(struct perf_event_attr));
+	pe.type = PERF_TYPE_BREAKPOINT;
+	pe.size = sizeof(struct perf_event_attr);
+
+	pe.config = 0;
+	pe.bp_type = HW_BREAKPOINT_X;
+	pe.bp_addr = (unsigned long) fn;
+	pe.bp_len = sizeof(long);
+
+	pe.sample_period = 1;
+	pe.sample_type = PERF_SAMPLE_IP;
+	pe.wakeup_events = 1;
+
+	pe.disabled = 1;
+	pe.exclude_kernel = 1;
+	pe.exclude_hv = 1;
+
+	fd = sys_perf_event_open(&pe, 0, -1, -1, 0);
+	if (fd < 0) {
+		pr_err("failed opening event %llx\n", pe.config);
+		return TEST_FAIL;
+	}
+
+	if (setup_signal) {
+		fcntl(fd, F_SETFL, O_RDWR|O_NONBLOCK|O_ASYNC);
+		fcntl(fd, F_SETSIG, SIGIO);
+		fcntl(fd, F_SETOWN, getpid());
+	}
+
+	ioctl(fd, PERF_EVENT_IOC_RESET, 0);
+
+	return fd;
+}
+
+static long long bp_count(int fd)
+{
+	long long count;
+	int ret;
+
+	ret = read(fd, &count, sizeof(long long));
+	if (ret != sizeof(long long)) {
+		pr_err("failed to read: %d\n", ret);
+		return TEST_FAIL;
+	}
+
+	return count;
+}
+
+int test__bp_signal(void)
+{
+	struct sigaction sa;
+	long long count1, count2;
+
+	/* setup SIGIO signal handler */
+	memset(&sa, 0, sizeof(struct sigaction));
+	sa.sa_sigaction = (void *) sig_handler;
+	sa.sa_flags = SA_SIGINFO;
+
+	if (sigaction(SIGIO, &sa, NULL) < 0) {
+		pr_err("failed setting up signal handler\n");
+		return TEST_FAIL;
+	}
+
+	/*
+	 * We create following events:
+	 *
+	 * fd1 - breakpoint event on test_function with SIGIO
+	 *       signal configured. We should get signal
+	 *       notification each time the breakpoint is hit
+	 *
+	 * fd2 - breakpoint event on sig_handler without SIGIO
+	 *       configured.
+	 *
+	 * Following processing should happen:
+	 *   - execute test_function
+	 *   - fd1 event breakpoint hit -> count1 == 1
+	 *   - SIGIO is delivered       -> overflows == 1
+	 *   - fd2 event breakpoint hit -> count2 == 1
+	 *
+	 * The test case check following error conditions:
+	 * - we get stuck in signal handler because of debug
+	 *   exception being triggered receursively due to
+	 *   the wrong RF EFLAG management
+	 *
+	 * - we never trigger the sig_handler breakpoint due
+	 *   to the rong RF EFLAG management
+	 *
+	 */
+
+	fd1 = bp_event(test_function, 1);
+	fd2 = bp_event(sig_handler, 0);
+
+	ioctl(fd1, PERF_EVENT_IOC_ENABLE, 0);
+	ioctl(fd2, PERF_EVENT_IOC_ENABLE, 0);
+
+	/*
+	 * Kick off the test by trigering 'fd1'
+	 * breakpoint.
+	 */
+	test_function();
+
+	ioctl(fd1, PERF_EVENT_IOC_DISABLE, 0);
+	ioctl(fd2, PERF_EVENT_IOC_DISABLE, 0);
+
+	count1 = bp_count(fd1);
+	count2 = bp_count(fd2);
+
+	close(fd1);
+	close(fd2);
+
+	pr_debug("count1 %lld, count2 %lld, overflow %d\n",
+		 count1, count2, overflows);
+
+	if (count1 != 1) {
+		if (count1 == 11)
+			pr_err("failed: RF EFLAG recursion issue detected\n");
+		else
+			pr_err("failed: wrong count for bp1%lld\n",
+			       count1);
+	}
+
+	if (overflows != 1)
+		pr_err("failed: wrong overflow hit\n");
+
+	if (count2 != 1)
+		pr_err("failed: wrong count for bp2\n");
+
+	return count1 == 1 && overflows == 1 && count2 == 1 ?
+		TEST_OK : TEST_FAIL;
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index acb98e0..37b108b 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -78,6 +78,10 @@ static struct test {
 		.func = test__python_use,
 	},
 	{
+		.desc = "Test breakpoint overflow signal handler",
+		.func = test__bp_signal,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 5de0be1..05d0e58 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -23,5 +23,6 @@ int test__dso_data(void);
 int test__parse_events(void);
 int test__hists_link(void);
 int test__python_use(void);
+int test__bp_signal(void);
 
 #endif /* TESTS_H */
-- 
1.7.11.7

[PATCH 5/5] perf tests: Test breakpoint overflow signal handler counts

[Jiri Olsa]

Adding automated test to check the exact number of
breakpoint event overflows and counts.

This test was originally done by Vince Weaver for
perf_event_tests.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
---
 tools/perf/Makefile                   |   1 +
 tools/perf/tests/bp_signal_overflow.c | 126 ++++++++++++++++++++++++++++++++++
 tools/perf/tests/builtin-test.c       |   4 ++
 tools/perf/tests/tests.h              |   1 +
 4 files changed, 132 insertions(+)
 create mode 100644 tools/perf/tests/bp_signal_overflow.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index bea66fe..34a63ce 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -498,6 +498,7 @@ LIB_OBJS += $(OUTPUT)tests/pmu.o
 LIB_OBJS += $(OUTPUT)tests/hists_link.o
 LIB_OBJS += $(OUTPUT)tests/python-use.o
 LIB_OBJS += $(OUTPUT)tests/bp_signal.o
+LIB_OBJS += $(OUTPUT)tests/bp_signal_overflow.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
diff --git a/tools/perf/tests/bp_signal_overflow.c b/tools/perf/tests/bp_signal_overflow.c
new file mode 100644
index 0000000..556d2ce
--- /dev/null
+++ b/tools/perf/tests/bp_signal_overflow.c
@@ -0,0 +1,126 @@
+/*
+ * Originally done by Vince Weaver <vincent.weaver@maine.edu> for
+ * perf_event_tests (git://github.com/deater/perf_event_tests)
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <time.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <linux/compiler.h>
+#include <linux/hw_breakpoint.h>
+
+#include "tests.h"
+#include "debug.h"
+#include "perf.h"
+
+static int overflows;
+
+__attribute__ ((noinline))
+static int test_function(void)
+{
+	return time(NULL);
+}
+
+static void sig_handler(int signum __maybe_unused,
+			siginfo_t *oh __maybe_unused,
+			void *uc __maybe_unused)
+{
+	overflows++;
+}
+
+static long long bp_count(int fd)
+{
+	long long count;
+	int ret;
+
+	ret = read(fd, &count, sizeof(long long));
+	if (ret != sizeof(long long)) {
+		pr_err("failed to read: %d\n", ret);
+		return TEST_FAIL;
+	}
+
+	return count;
+}
+
+#define EXECUTIONS 10000
+#define THRESHOLD  100
+
+int test__bp_signal_overflow(void)
+{
+	struct perf_event_attr pe;
+	struct sigaction sa;
+	long long count;
+	int fd, i, fails = 0;
+
+	/* setup SIGIO signal handler */
+	memset(&sa, 0, sizeof(struct sigaction));
+	sa.sa_sigaction = (void *) sig_handler;
+	sa.sa_flags = SA_SIGINFO;
+
+	if (sigaction(SIGIO, &sa, NULL) < 0) {
+		pr_err("failed setting up signal handler\n");
+		return TEST_FAIL;
+	}
+
+	memset(&pe, 0, sizeof(struct perf_event_attr));
+	pe.type = PERF_TYPE_BREAKPOINT;
+	pe.size = sizeof(struct perf_event_attr);
+
+	pe.config = 0;
+	pe.bp_type = HW_BREAKPOINT_X;
+	pe.bp_addr = (unsigned long) test_function;
+	pe.bp_len = sizeof(long);
+
+	pe.sample_period = THRESHOLD;
+	pe.sample_type = PERF_SAMPLE_IP;
+	pe.wakeup_events = 1;
+
+	pe.disabled = 1;
+	pe.exclude_kernel = 1;
+	pe.exclude_hv = 1;
+
+	fd = sys_perf_event_open(&pe, 0, -1, -1, 0);
+	if (fd < 0) {
+		pr_err("failed opening event %llx\n", pe.config);
+		return TEST_FAIL;
+	}
+
+	fcntl(fd, F_SETFL, O_RDWR|O_NONBLOCK|O_ASYNC);
+	fcntl(fd, F_SETSIG, SIGIO);
+	fcntl(fd, F_SETOWN, getpid());
+
+	ioctl(fd, PERF_EVENT_IOC_RESET, 0);
+	ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);
+
+	for (i = 0; i < EXECUTIONS; i++)
+		test_function();
+
+	ioctl(fd, PERF_EVENT_IOC_DISABLE, 0);
+
+	count = bp_count(fd);
+
+	close(fd);
+
+	pr_debug("count %lld, overflow %d\n",
+		 count, overflows);
+
+	if (count != EXECUTIONS) {
+		pr_err("\tWrong number of executions %lld != %d\n",
+		count, EXECUTIONS);
+		fails++;
+	}
+
+	if (overflows != EXECUTIONS / THRESHOLD) {
+		pr_err("\tWrong number of overflows %d != %d\n",
+		overflows, EXECUTIONS / THRESHOLD);
+		fails++;
+	}
+
+	return fails ? TEST_FAIL : TEST_OK;
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 37b108b..45d9ad4 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -82,6 +82,10 @@ static struct test {
 		.func = test__bp_signal,
 	},
 	{
+		.desc = "Test breakpoint overflow sampling",
+		.func = test__bp_signal_overflow,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 05d0e58..6cf1ec4 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -24,5 +24,6 @@ int test__parse_events(void);
 int test__hists_link(void);
 int test__python_use(void);
 int test__bp_signal(void);
+int test__bp_signal_overflow(void);
 
 #endif /* TESTS_H */
-- 
1.7.11.7

Re: [PATCH 2/5] signal x86: Clear RF EFLAGS bit for signal handler

[H. Peter Anvin]

On 03/01/2013 10:11 AM, Jiri Olsa wrote:
> Clearing RF EFLAGS bit for signal handler. The reason is,
> that this flag is set by debug exception code to prevent
> the recursive exception entry.
> 
> Leaving it set for signal handler might prevent debug
> exception of the signal handler itself.
> 
> diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> index b6fe116..e273571 100644
> --- a/arch/x86/kernel/signal.c
> +++ b/arch/x86/kernel/signal.c
> @@ -726,6 +726,13 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
>  	regs->flags &= ~X86_EFLAGS_DF;
>  
>  	/*
> +	 * Clear RF when entering the signal handler, because
> +	 * it might disable possible debug exception from the
> +	 * signal handler.
> +	 */
> +	regs->flags &= ~X86_EFLAGS_RF;
> +
> +	/*
>  	 * Clear TF when entering the signal handler, but
>  	 * notify any tracer that was single-stepping it.
>  	 * The tracer may want to single-step inside the
> 

Makes sense.  However, can you combine all the flags-clearing into one
statement while you are at it?

	-hpa

Re: [PATCH 2/5] signal x86: Clear RF EFLAGS bit for signal handler

[Jiri Olsa]

On Fri, Mar 01, 2013 at 10:57:50AM -0800, H. Peter Anvin wrote:
> On 03/01/2013 10:11 AM, Jiri Olsa wrote:
> > Clearing RF EFLAGS bit for signal handler. The reason is,
> > that this flag is set by debug exception code to prevent
> > the recursive exception entry.
> > 
> > Leaving it set for signal handler might prevent debug
> > exception of the signal handler itself.
> > 
> > diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> > index b6fe116..e273571 100644
> > --- a/arch/x86/kernel/signal.c
> > +++ b/arch/x86/kernel/signal.c
> > @@ -726,6 +726,13 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
> >  	regs->flags &= ~X86_EFLAGS_DF;
> >  
> >  	/*
> > +	 * Clear RF when entering the signal handler, because
> > +	 * it might disable possible debug exception from the
> > +	 * signal handler.
> > +	 */
> > +	regs->flags &= ~X86_EFLAGS_RF;
> > +
> > +	/*
> >  	 * Clear TF when entering the signal handler, but
> >  	 * notify any tracer that was single-stepping it.
> >  	 * The tracer may want to single-step inside the
> > 
> 
> Makes sense.  However, can you combine all the flags-clearing into one
> statement while you are at it?

sure, will send v2

thanks,
jirka

Re: [PATCH 2/5] signal x86: Clear RF EFLAGS bit for signal handler

[Jiri Olsa]

On Sun, Mar 03, 2013 at 11:43:21AM +0100, Jiri Olsa wrote:
> On Fri, Mar 01, 2013 at 10:57:50AM -0800, H. Peter Anvin wrote:
> > On 03/01/2013 10:11 AM, Jiri Olsa wrote:
> > > Clearing RF EFLAGS bit for signal handler. The reason is,
> > > that this flag is set by debug exception code to prevent
> > > the recursive exception entry.
> > > 
> > > Leaving it set for signal handler might prevent debug
> > > exception of the signal handler itself.
> > > 
> > > diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> > > index b6fe116..e273571 100644
> > > --- a/arch/x86/kernel/signal.c
> > > +++ b/arch/x86/kernel/signal.c
> > > @@ -726,6 +726,13 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
> > >  	regs->flags &= ~X86_EFLAGS_DF;
> > >  
> > >  	/*
> > > +	 * Clear RF when entering the signal handler, because
> > > +	 * it might disable possible debug exception from the
> > > +	 * signal handler.
> > > +	 */
> > > +	regs->flags &= ~X86_EFLAGS_RF;
> > > +
> > > +	/*
> > >  	 * Clear TF when entering the signal handler, but
> > >  	 * notify any tracer that was single-stepping it.
> > >  	 * The tracer may want to single-step inside the
> > > 
> > 
> > Makes sense.  However, can you combine all the flags-clearing into one
> > statement while you are at it?

Is there any other reason for this besides having just signal
instruction doing this update?

It looks like gcc is smart enough to do that anyway:

...
andq   $0xfffffffffffffaff,(%rdi)
...


I need to send V2 anyway, since the patchset no longer
applies into latest tip.

thanks,
jirka

Re: [PATCH 2/5] signal x86: Clear RF EFLAGS bit for signal handler

[Jiri Olsa]

On Sun, Mar 10, 2013 at 06:15:18PM +0100, Jiri Olsa wrote:
> On Sun, Mar 03, 2013 at 11:43:21AM +0100, Jiri Olsa wrote:
> > On Fri, Mar 01, 2013 at 10:57:50AM -0800, H. Peter Anvin wrote:
> > > On 03/01/2013 10:11 AM, Jiri Olsa wrote:
> > > > Clearing RF EFLAGS bit for signal handler. The reason is,
> > > > that this flag is set by debug exception code to prevent
> > > > the recursive exception entry.
> > > > 
> > > > Leaving it set for signal handler might prevent debug
> > > > exception of the signal handler itself.
> > > > 
> > > > diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> > > > index b6fe116..e273571 100644
> > > > --- a/arch/x86/kernel/signal.c
> > > > +++ b/arch/x86/kernel/signal.c
> > > > @@ -726,6 +726,13 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
> > > >  	regs->flags &= ~X86_EFLAGS_DF;
> > > >  
> > > >  	/*
> > > > +	 * Clear RF when entering the signal handler, because
> > > > +	 * it might disable possible debug exception from the
> > > > +	 * signal handler.
> > > > +	 */
> > > > +	regs->flags &= ~X86_EFLAGS_RF;
> > > > +
> > > > +	/*
> > > >  	 * Clear TF when entering the signal handler, but
> > > >  	 * notify any tracer that was single-stepping it.
> > > >  	 * The tracer may want to single-step inside the
> > > > 
> > > 
> > > Makes sense.  However, can you combine all the flags-clearing into one
> > > statement while you are at it?
> 
> Is there any other reason for this besides having just signal

s/signal/single/ ;-)

jirka
> instruction doing this update?
> 
> It looks like gcc is smart enough to do that anyway:
> 
> ...
> andq   $0xfffffffffffffaff,(%rdi)
> ...
> 
> 
> I need to send V2 anyway, since the patchset no longer
> applies into latest tip.
> 
> thanks,
> jirka