[PATCHv3 0/8] hw_breakpoint: Breakpoint modification fixes and new modify ioctl

[PATCHv3 0/8] hw_breakpoint: Breakpoint modification fixes and new modify ioctl

[Jiri Olsa]

hi,
Milind Chabbi introduced new ioctl interface to change
live breakpoint [1]. It allows to change its bp_addr,
bp_len and bp_type throught new ioctl for perf breakpoint
event.

We already have a kernel interface for this via 
modify_user_hw_breakpoint function. This function however
does not update the breakpoint slot counts.

So when the same functionality was exposed to user space
(Milind's change), with simple test program I could put wrong
slots count on arm server [2] and ended up with no breakpoints
available on the system. Note it's not an issue on x86, because
it shares slot single counter for both data and inst types
(CONFIG_HAVE_MIXED_BREAKPOINTS_REGS).

This patchset contains my fixes for keeping breakpoint slots
count updated. On top of it there's Milind's change for new
_IOC_MODIFY_ATTRIBUTES ioctl interface to change a breakpoint.

As I mentioned above there're kernel users of
modify_user_hw_breakpoint function, all ptrace related
AFAICS, which could got broken.. so cc-ing Oleg ;-)

I ran gdb and strace tests suites and got same amount of
skip/fail tests as when I run them on unpatched machine,
so I assume nothing new got broken.

It's also available in here:
  https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  perf/bp

v3 changes:
  - rebased on Arnaldo's perf/core
  - minor format changes in PERF_EVENT_IOC_* hunks

v2 changes:
  - added check for the rest of the perf_event_attr fields
    to be the same as for kernel event

thanks,
jirka


[1] https://marc.info/?l=linux-kernel&m=151012255331565&w=2
[2] https://marc.info/?l=linux-man&m=151172469807302&w=2
---
Jiri Olsa (7):
      hw_breakpoint: Pass bp_type directly as find_slot_idx argument
      hw_breakpoint: Pass bp_type argument to __reserve_bp_slot|__release_bp_slot
      hw_breakpoint: Add modify_bp_slot function
      hw_breakpoint: Factor out __modify_user_hw_breakpoint function
      hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint
      perf/core: Move perf_event_attr::sample_max_stack into perf_copy_attr
      perf tests: Add breakpoint accounting/modify test

Milind Chabbi (1):
      perf/core: fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES.

 include/linux/hw_breakpoint.h         |   7 +++++
 include/uapi/linux/perf_event.h       |  23 ++++++++-------
 kernel/events/core.c                  |  54 ++++++++++++++++++++++++++++++++--
 kernel/events/hw_breakpoint.c         | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++---------------------
 tools/include/uapi/linux/perf_event.h |  23 ++++++++-------
 tools/perf/tests/Build                |   1 +
 tools/perf/tests/bp_account.c         | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/builtin-test.c       |   4 +++
 tools/perf/tests/tests.h              |   1 +
 9 files changed, 365 insertions(+), 58 deletions(-)
 create mode 100644 tools/perf/tests/bp_account.c

[PATCH 1/8] hw_breakpoint: Pass bp_type directly as find_slot_idx argument

[Jiri Olsa]

Passing bp_type directly as find_slot_idx argument,
so we don't need to have whole event to get the
breakpoint slot type. It will be used in following
changes.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/events/hw_breakpoint.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 3f8cb1e14588..395ca07965af 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -85,9 +85,9 @@ __weak int hw_breakpoint_weight(struct perf_event *bp)
 	return 1;
 }
 
-static inline enum bp_type_idx find_slot_idx(struct perf_event *bp)
+static inline enum bp_type_idx find_slot_idx(u64 bp_type)
 {
-	if (bp->attr.bp_type & HW_BREAKPOINT_RW)
+	if (bp_type & HW_BREAKPOINT_RW)
 		return TYPE_DATA;
 
 	return TYPE_INST;
@@ -122,7 +122,7 @@ static int task_bp_pinned(int cpu, struct perf_event *bp, enum bp_type_idx type)
 
 	list_for_each_entry(iter, &bp_task_head, hw.bp_list) {
 		if (iter->hw.target == tsk &&
-		    find_slot_idx(iter) == type &&
+		    find_slot_idx(iter->attr.bp_type) == type &&
 		    (iter->cpu < 0 || cpu == iter->cpu))
 			count += hw_breakpoint_weight(iter);
 	}
@@ -292,7 +292,7 @@ static int __reserve_bp_slot(struct perf_event *bp)
 	    bp->attr.bp_type == HW_BREAKPOINT_INVALID)
 		return -EINVAL;
 
-	type = find_slot_idx(bp);
+	type = find_slot_idx(bp->attr.bp_type);
 	weight = hw_breakpoint_weight(bp);
 
 	fetch_bp_busy_slots(&slots, bp, type);
@@ -329,7 +329,7 @@ static void __release_bp_slot(struct perf_event *bp)
 	enum bp_type_idx type;
 	int weight;
 
-	type = find_slot_idx(bp);
+	type = find_slot_idx(bp->attr.bp_type);
 	weight = hw_breakpoint_weight(bp);
 	toggle_bp_slot(bp, false, type, weight);
 }
-- 
2.13.6

[tip:perf/core] hw_breakpoint: Pass bp_type directly as find_slot_idx() argument

[tip-bot for Jiri Olsa]

Commit-ID:  cbd9d9f114f9e5931ba199f4450667656b892f32
Gitweb:     https://git.kernel.org/tip/cbd9d9f114f9e5931ba199f4450667656b892f32
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 12 Mar 2018 14:45:41 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Mar 2018 06:56:07 +0100

hw_breakpoint: Pass bp_type directly as find_slot_idx() argument

Pass bp_type directly as a find_slot_idx() argument,
so we don't need to have whole event to get the
breakpoint slot type. It will be used in following
changes.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Milind Chabbi <chabbi.milind@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20180312134548.31532-2-jolsa@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/hw_breakpoint.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 3f8cb1e14588..395ca07965af 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -85,9 +85,9 @@ __weak int hw_breakpoint_weight(struct perf_event *bp)
 	return 1;
 }
 
-static inline enum bp_type_idx find_slot_idx(struct perf_event *bp)
+static inline enum bp_type_idx find_slot_idx(u64 bp_type)
 {
-	if (bp->attr.bp_type & HW_BREAKPOINT_RW)
+	if (bp_type & HW_BREAKPOINT_RW)
 		return TYPE_DATA;
 
 	return TYPE_INST;
@@ -122,7 +122,7 @@ static int task_bp_pinned(int cpu, struct perf_event *bp, enum bp_type_idx type)
 
 	list_for_each_entry(iter, &bp_task_head, hw.bp_list) {
 		if (iter->hw.target == tsk &&
-		    find_slot_idx(iter) == type &&
+		    find_slot_idx(iter->attr.bp_type) == type &&
 		    (iter->cpu < 0 || cpu == iter->cpu))
 			count += hw_breakpoint_weight(iter);
 	}
@@ -292,7 +292,7 @@ static int __reserve_bp_slot(struct perf_event *bp)
 	    bp->attr.bp_type == HW_BREAKPOINT_INVALID)
 		return -EINVAL;
 
-	type = find_slot_idx(bp);
+	type = find_slot_idx(bp->attr.bp_type);
 	weight = hw_breakpoint_weight(bp);
 
 	fetch_bp_busy_slots(&slots, bp, type);
@@ -329,7 +329,7 @@ static void __release_bp_slot(struct perf_event *bp)
 	enum bp_type_idx type;
 	int weight;
 
-	type = find_slot_idx(bp);
+	type = find_slot_idx(bp->attr.bp_type);
 	weight = hw_breakpoint_weight(bp);
 	toggle_bp_slot(bp, false, type, weight);
 }

[PATCH 2/8] hw_breakpoint: Pass bp_type argument to __reserve_bp_slot|__release_bp_slot

[Jiri Olsa]

Passing bp_type argument to __reserve_bp_slot and __release_bp_slot
functions, so we can pass another bp_type than the one defined in
bp->attr.bp_type. This will be handy in following change that fixes
breakpoint slot counts during its modification.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/events/hw_breakpoint.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 395ca07965af..9b31fbcc3305 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -277,7 +277,7 @@ __weak void arch_unregister_hw_breakpoint(struct perf_event *bp)
  *       ((per_cpu(info->flexible, *) > 1) + max(per_cpu(info->cpu_pinned, *))
  *            + max(per_cpu(info->tsk_pinned, *))) < HBP_NUM
  */
-static int __reserve_bp_slot(struct perf_event *bp)
+static int __reserve_bp_slot(struct perf_event *bp, u64 bp_type)
 {
 	struct bp_busy_slots slots = {0};
 	enum bp_type_idx type;
@@ -288,11 +288,11 @@ static int __reserve_bp_slot(struct perf_event *bp)
 		return -ENOMEM;
 
 	/* Basic checks */
-	if (bp->attr.bp_type == HW_BREAKPOINT_EMPTY ||
-	    bp->attr.bp_type == HW_BREAKPOINT_INVALID)
+	if (bp_type == HW_BREAKPOINT_EMPTY ||
+	    bp_type == HW_BREAKPOINT_INVALID)
 		return -EINVAL;
 
-	type = find_slot_idx(bp->attr.bp_type);
+	type = find_slot_idx(bp_type);
 	weight = hw_breakpoint_weight(bp);
 
 	fetch_bp_busy_slots(&slots, bp, type);
@@ -317,19 +317,19 @@ int reserve_bp_slot(struct perf_event *bp)
 
 	mutex_lock(&nr_bp_mutex);
 
-	ret = __reserve_bp_slot(bp);
+	ret = __reserve_bp_slot(bp, bp->attr.bp_type);
 
 	mutex_unlock(&nr_bp_mutex);
 
 	return ret;
 }
 
-static void __release_bp_slot(struct perf_event *bp)
+static void __release_bp_slot(struct perf_event *bp, u64 bp_type)
 {
 	enum bp_type_idx type;
 	int weight;
 
-	type = find_slot_idx(bp->attr.bp_type);
+	type = find_slot_idx(bp_type);
 	weight = hw_breakpoint_weight(bp);
 	toggle_bp_slot(bp, false, type, weight);
 }
@@ -339,7 +339,7 @@ void release_bp_slot(struct perf_event *bp)
 	mutex_lock(&nr_bp_mutex);
 
 	arch_unregister_hw_breakpoint(bp);
-	__release_bp_slot(bp);
+	__release_bp_slot(bp, bp->attr.bp_type);
 
 	mutex_unlock(&nr_bp_mutex);
 }
@@ -354,7 +354,7 @@ int dbg_reserve_bp_slot(struct perf_event *bp)
 	if (mutex_is_locked(&nr_bp_mutex))
 		return -1;
 
-	return __reserve_bp_slot(bp);
+	return __reserve_bp_slot(bp, bp->attr.bp_type);
 }
 
 int dbg_release_bp_slot(struct perf_event *bp)
@@ -362,7 +362,7 @@ int dbg_release_bp_slot(struct perf_event *bp)
 	if (mutex_is_locked(&nr_bp_mutex))
 		return -1;
 
-	__release_bp_slot(bp);
+	__release_bp_slot(bp, bp->attr.bp_type);
 
 	return 0;
 }
-- 
2.13.6

[tip:perf/core] hw_breakpoint: Pass bp_type argument to __reserve_bp_slot|__release_bp_slot()

[tip-bot for Jiri Olsa]

Commit-ID:  1ad9ff7dea4c42bee43f98b7d7ce8037ee22e133
Gitweb:     https://git.kernel.org/tip/1ad9ff7dea4c42bee43f98b7d7ce8037ee22e133
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 12 Mar 2018 14:45:42 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Mar 2018 06:56:07 +0100

hw_breakpoint: Pass bp_type argument to __reserve_bp_slot|__release_bp_slot()

Passing bp_type argument to __reserve_bp_slot() and __release_bp_slot()
functions, so we can pass another bp_type than the one defined in
bp->attr.bp_type. This will be handy in following change that fixes
breakpoint slot counts during its modification.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Milind Chabbi <chabbi.milind@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20180312134548.31532-3-jolsa@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/hw_breakpoint.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 395ca07965af..9b31fbcc3305 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -277,7 +277,7 @@ __weak void arch_unregister_hw_breakpoint(struct perf_event *bp)
  *       ((per_cpu(info->flexible, *) > 1) + max(per_cpu(info->cpu_pinned, *))
  *            + max(per_cpu(info->tsk_pinned, *))) < HBP_NUM
  */
-static int __reserve_bp_slot(struct perf_event *bp)
+static int __reserve_bp_slot(struct perf_event *bp, u64 bp_type)
 {
 	struct bp_busy_slots slots = {0};
 	enum bp_type_idx type;
@@ -288,11 +288,11 @@ static int __reserve_bp_slot(struct perf_event *bp)
 		return -ENOMEM;
 
 	/* Basic checks */
-	if (bp->attr.bp_type == HW_BREAKPOINT_EMPTY ||
-	    bp->attr.bp_type == HW_BREAKPOINT_INVALID)
+	if (bp_type == HW_BREAKPOINT_EMPTY ||
+	    bp_type == HW_BREAKPOINT_INVALID)
 		return -EINVAL;
 
-	type = find_slot_idx(bp->attr.bp_type);
+	type = find_slot_idx(bp_type);
 	weight = hw_breakpoint_weight(bp);
 
 	fetch_bp_busy_slots(&slots, bp, type);
@@ -317,19 +317,19 @@ int reserve_bp_slot(struct perf_event *bp)
 
 	mutex_lock(&nr_bp_mutex);
 
-	ret = __reserve_bp_slot(bp);
+	ret = __reserve_bp_slot(bp, bp->attr.bp_type);
 
 	mutex_unlock(&nr_bp_mutex);
 
 	return ret;
 }
 
-static void __release_bp_slot(struct perf_event *bp)
+static void __release_bp_slot(struct perf_event *bp, u64 bp_type)
 {
 	enum bp_type_idx type;
 	int weight;
 
-	type = find_slot_idx(bp->attr.bp_type);
+	type = find_slot_idx(bp_type);
 	weight = hw_breakpoint_weight(bp);
 	toggle_bp_slot(bp, false, type, weight);
 }
@@ -339,7 +339,7 @@ void release_bp_slot(struct perf_event *bp)
 	mutex_lock(&nr_bp_mutex);
 
 	arch_unregister_hw_breakpoint(bp);
-	__release_bp_slot(bp);
+	__release_bp_slot(bp, bp->attr.bp_type);
 
 	mutex_unlock(&nr_bp_mutex);
 }
@@ -354,7 +354,7 @@ int dbg_reserve_bp_slot(struct perf_event *bp)
 	if (mutex_is_locked(&nr_bp_mutex))
 		return -1;
 
-	return __reserve_bp_slot(bp);
+	return __reserve_bp_slot(bp, bp->attr.bp_type);
 }
 
 int dbg_release_bp_slot(struct perf_event *bp)
@@ -362,7 +362,7 @@ int dbg_release_bp_slot(struct perf_event *bp)
 	if (mutex_is_locked(&nr_bp_mutex))
 		return -1;
 
-	__release_bp_slot(bp);
+	__release_bp_slot(bp, bp->attr.bp_type);
 
 	return 0;
 }

[PATCH 3/8] hw_breakpoint: Add modify_bp_slot function

[Jiri Olsa]

Adding modify_bp_slot function to keep slot numbers
correct when changing the breakpoint type.

Using existing __release_bp_slot/__reserve_bp_slot
call sequence to update the slot counts.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/events/hw_breakpoint.c | 46 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 9b31fbcc3305..776948beb4ac 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -44,6 +44,7 @@
 #include <linux/list.h>
 #include <linux/cpu.h>
 #include <linux/smp.h>
+#include <linux/bug.h>
 
 #include <linux/hw_breakpoint.h>
 /*
@@ -344,6 +345,38 @@ void release_bp_slot(struct perf_event *bp)
 	mutex_unlock(&nr_bp_mutex);
 }
 
+static int __modify_bp_slot(struct perf_event *bp, u64 old_type)
+{
+	int err;
+
+	__release_bp_slot(bp, old_type);
+
+	err = __reserve_bp_slot(bp, bp->attr.bp_type);
+	if (err) {
+		/*
+		 * Reserve the old_type slot back in case
+		 * there's no space for the new type.
+		 *
+		 * This must succeed, because we just released
+		 * the old_type slot in the __release_bp_slot
+		 * call above. If not, something is broken.
+		 */
+		WARN_ON(__reserve_bp_slot(bp, old_type));
+	}
+
+	return err;
+}
+
+static int modify_bp_slot(struct perf_event *bp, u64 old_type)
+{
+	int ret;
+
+	mutex_lock(&nr_bp_mutex);
+	ret = __modify_bp_slot(bp, old_type);
+	mutex_unlock(&nr_bp_mutex);
+	return ret;
+}
+
 /*
  * Allow the kernel debugger to reserve breakpoint slots without
  * taking a lock using the dbg_* variant of for the reserve and
@@ -435,6 +468,7 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 	u64 old_addr = bp->attr.bp_addr;
 	u64 old_len = bp->attr.bp_len;
 	int old_type = bp->attr.bp_type;
+	bool modify = attr->bp_type != old_type;
 	int err = 0;
 
 	/*
@@ -452,12 +486,9 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 	bp->attr.bp_type = attr->bp_type;
 	bp->attr.bp_len = attr->bp_len;
 
-	if (attr->disabled)
-		goto end;
-
 	err = validate_hw_breakpoint(bp);
-	if (!err)
-		perf_event_enable(bp);
+	if (!err && modify)
+		err = modify_bp_slot(bp, old_type);
 
 	if (err) {
 		bp->attr.bp_addr = old_addr;
@@ -469,9 +500,10 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 		return err;
 	}
 
-end:
-	bp->attr.disabled = attr->disabled;
+	if (!attr->disabled)
+		perf_event_enable(bp);
 
+	bp->attr.disabled = attr->disabled;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(modify_user_hw_breakpoint);
-- 
2.13.6

[tip:perf/core] hw_breakpoint: Add modify_bp_slot() function

[tip-bot for Jiri Olsa]

Commit-ID:  ea6a9d530c1779b9bd30944b803820c462f01eac
Gitweb:     https://git.kernel.org/tip/ea6a9d530c1779b9bd30944b803820c462f01eac
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 12 Mar 2018 14:45:43 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Mar 2018 06:56:07 +0100

hw_breakpoint: Add modify_bp_slot() function

Add the modify_bp_slot() function to keep slot numbers
correct when changing the breakpoint type.

Using existing __release_bp_slot()/__reserve_bp_slot()
call sequence to update the slot counts.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Milind Chabbi <chabbi.milind@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20180312134548.31532-4-jolsa@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/hw_breakpoint.c | 46 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 9b31fbcc3305..776948beb4ac 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -44,6 +44,7 @@
 #include <linux/list.h>
 #include <linux/cpu.h>
 #include <linux/smp.h>
+#include <linux/bug.h>
 
 #include <linux/hw_breakpoint.h>
 /*
@@ -344,6 +345,38 @@ void release_bp_slot(struct perf_event *bp)
 	mutex_unlock(&nr_bp_mutex);
 }
 
+static int __modify_bp_slot(struct perf_event *bp, u64 old_type)
+{
+	int err;
+
+	__release_bp_slot(bp, old_type);
+
+	err = __reserve_bp_slot(bp, bp->attr.bp_type);
+	if (err) {
+		/*
+		 * Reserve the old_type slot back in case
+		 * there's no space for the new type.
+		 *
+		 * This must succeed, because we just released
+		 * the old_type slot in the __release_bp_slot
+		 * call above. If not, something is broken.
+		 */
+		WARN_ON(__reserve_bp_slot(bp, old_type));
+	}
+
+	return err;
+}
+
+static int modify_bp_slot(struct perf_event *bp, u64 old_type)
+{
+	int ret;
+
+	mutex_lock(&nr_bp_mutex);
+	ret = __modify_bp_slot(bp, old_type);
+	mutex_unlock(&nr_bp_mutex);
+	return ret;
+}
+
 /*
  * Allow the kernel debugger to reserve breakpoint slots without
  * taking a lock using the dbg_* variant of for the reserve and
@@ -435,6 +468,7 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 	u64 old_addr = bp->attr.bp_addr;
 	u64 old_len = bp->attr.bp_len;
 	int old_type = bp->attr.bp_type;
+	bool modify = attr->bp_type != old_type;
 	int err = 0;
 
 	/*
@@ -452,12 +486,9 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 	bp->attr.bp_type = attr->bp_type;
 	bp->attr.bp_len = attr->bp_len;
 
-	if (attr->disabled)
-		goto end;
-
 	err = validate_hw_breakpoint(bp);
-	if (!err)
-		perf_event_enable(bp);
+	if (!err && modify)
+		err = modify_bp_slot(bp, old_type);
 
 	if (err) {
 		bp->attr.bp_addr = old_addr;
@@ -469,9 +500,10 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 		return err;
 	}
 
-end:
-	bp->attr.disabled = attr->disabled;
+	if (!attr->disabled)
+		perf_event_enable(bp);
 
+	bp->attr.disabled = attr->disabled;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(modify_user_hw_breakpoint);

[PATCH 4/8] hw_breakpoint: Factor out __modify_user_hw_breakpoint function

[Jiri Olsa]

Moving out the all the functionality without the events
disabling/enabling calls, because we want to call another
disabling/enabling functions in following change.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/events/hw_breakpoint.c | 46 +++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 776948beb4ac..a556aba223da 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -456,6 +456,33 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 }
 EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
 
+static int __modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
+{
+	u64 old_addr = bp->attr.bp_addr;
+	u64 old_len  = bp->attr.bp_len;
+	int old_type = bp->attr.bp_type;
+	bool modify  = attr->bp_type != old_type;
+	int err = 0;
+
+	bp->attr.bp_addr = attr->bp_addr;
+	bp->attr.bp_type = attr->bp_type;
+	bp->attr.bp_len  = attr->bp_len;
+
+	err = validate_hw_breakpoint(bp);
+	if (!err && modify)
+		err = modify_bp_slot(bp, old_type);
+
+	if (err) {
+		bp->attr.bp_addr = old_addr;
+		bp->attr.bp_type = old_type;
+		bp->attr.bp_len  = old_len;
+		return err;
+	}
+
+	bp->attr.disabled = attr->disabled;
+	return 0;
+}
+
 /**
  * modify_user_hw_breakpoint - modify a user-space hardware breakpoint
  * @bp: the breakpoint structure to modify
@@ -465,11 +492,7 @@ EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
  */
 int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
 {
-	u64 old_addr = bp->attr.bp_addr;
-	u64 old_len = bp->attr.bp_len;
-	int old_type = bp->attr.bp_type;
-	bool modify = attr->bp_type != old_type;
-	int err = 0;
+	int err;
 
 	/*
 	 * modify_user_hw_breakpoint can be invoked with IRQs disabled and hence it
@@ -482,18 +505,9 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 	else
 		perf_event_disable(bp);
 
-	bp->attr.bp_addr = attr->bp_addr;
-	bp->attr.bp_type = attr->bp_type;
-	bp->attr.bp_len = attr->bp_len;
-
-	err = validate_hw_breakpoint(bp);
-	if (!err && modify)
-		err = modify_bp_slot(bp, old_type);
+	err = __modify_user_hw_breakpoint(bp, attr);
 
 	if (err) {
-		bp->attr.bp_addr = old_addr;
-		bp->attr.bp_type = old_type;
-		bp->attr.bp_len = old_len;
 		if (!bp->attr.disabled)
 			perf_event_enable(bp);
 
@@ -502,8 +516,6 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 
 	if (!attr->disabled)
 		perf_event_enable(bp);
-
-	bp->attr.disabled = attr->disabled;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(modify_user_hw_breakpoint);
-- 
2.13.6

[tip:perf/core] hw_breakpoint: Factor out __modify_user_hw_breakpoint() function

[tip-bot for Jiri Olsa]

Commit-ID:  18ff57b220610a699947f20b156a8245ca7eee98
Gitweb:     https://git.kernel.org/tip/18ff57b220610a699947f20b156a8245ca7eee98
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 12 Mar 2018 14:45:44 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Mar 2018 06:56:08 +0100

hw_breakpoint: Factor out __modify_user_hw_breakpoint() function

Moving out all the functionality without the events
disabling/enabling calls, because we want to call another
disabling/enabling functions in following change.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Milind Chabbi <chabbi.milind@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20180312134548.31532-5-jolsa@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/hw_breakpoint.c | 46 +++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 776948beb4ac..a556aba223da 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -456,6 +456,33 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 }
 EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
 
+static int __modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
+{
+	u64 old_addr = bp->attr.bp_addr;
+	u64 old_len  = bp->attr.bp_len;
+	int old_type = bp->attr.bp_type;
+	bool modify  = attr->bp_type != old_type;
+	int err = 0;
+
+	bp->attr.bp_addr = attr->bp_addr;
+	bp->attr.bp_type = attr->bp_type;
+	bp->attr.bp_len  = attr->bp_len;
+
+	err = validate_hw_breakpoint(bp);
+	if (!err && modify)
+		err = modify_bp_slot(bp, old_type);
+
+	if (err) {
+		bp->attr.bp_addr = old_addr;
+		bp->attr.bp_type = old_type;
+		bp->attr.bp_len  = old_len;
+		return err;
+	}
+
+	bp->attr.disabled = attr->disabled;
+	return 0;
+}
+
 /**
  * modify_user_hw_breakpoint - modify a user-space hardware breakpoint
  * @bp: the breakpoint structure to modify
@@ -465,11 +492,7 @@ EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
  */
 int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
 {
-	u64 old_addr = bp->attr.bp_addr;
-	u64 old_len = bp->attr.bp_len;
-	int old_type = bp->attr.bp_type;
-	bool modify = attr->bp_type != old_type;
-	int err = 0;
+	int err;
 
 	/*
 	 * modify_user_hw_breakpoint can be invoked with IRQs disabled and hence it
@@ -482,18 +505,9 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 	else
 		perf_event_disable(bp);
 
-	bp->attr.bp_addr = attr->bp_addr;
-	bp->attr.bp_type = attr->bp_type;
-	bp->attr.bp_len = attr->bp_len;
-
-	err = validate_hw_breakpoint(bp);
-	if (!err && modify)
-		err = modify_bp_slot(bp, old_type);
+	err = __modify_user_hw_breakpoint(bp, attr);
 
 	if (err) {
-		bp->attr.bp_addr = old_addr;
-		bp->attr.bp_type = old_type;
-		bp->attr.bp_len = old_len;
 		if (!bp->attr.disabled)
 			perf_event_enable(bp);
 
@@ -502,8 +516,6 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 
 	if (!attr->disabled)
 		perf_event_enable(bp);
-
-	bp->attr.disabled = attr->disabled;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(modify_user_hw_breakpoint);

[PATCH 5/8] hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint

[Jiri Olsa]

And rename it to modify_user_hw_breakpoint_check.

We are about to use modify_user_hw_breakpoint_check for user space
breakpoints modification, we must be very strict to check only the
fields we can change have changed. As Peter explained:

  Suppose someone does:
        attr = malloc(sizeof(*attr)); // uninitialized memory
        attr->type = BP;
        attr->bp_addr = new_addr;
        attr->bp_type = bp_type;
        attr->bp_len = bp_len;
        ioctl(fd, PERF_IOC_MOD_ATTR, &attr);

  And feeds absolute shite for the rest of the fields.
  Then we later want to extend IOC_MOD_ATTR to allow changing
  attr::sample_type but we can't, because that would break the
  above application.

I'm making this check optional because we already export
modify_user_hw_breakpoint and with this check we could
break existing users.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/events/hw_breakpoint.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index a556aba223da..0c82663395f7 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -456,7 +456,9 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 }
 EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
 
-static int __modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
+static int
+modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
+			        bool check)
 {
 	u64 old_addr = bp->attr.bp_addr;
 	u64 old_len  = bp->attr.bp_len;
@@ -468,6 +470,9 @@ static int __modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_
 	bp->attr.bp_type = attr->bp_type;
 	bp->attr.bp_len  = attr->bp_len;
 
+	if (check && memcmp(&bp->attr, attr, sizeof(*attr)))
+		return -EINVAL;
+
 	err = validate_hw_breakpoint(bp);
 	if (!err && modify)
 		err = modify_bp_slot(bp, old_type);
@@ -505,7 +510,7 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 	else
 		perf_event_disable(bp);
 
-	err = __modify_user_hw_breakpoint(bp, attr);
+	err = modify_user_hw_breakpoint_check(bp, attr, false);
 
 	if (err) {
 		if (!bp->attr.disabled)
-- 
2.13.6

[tip:perf/core] hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint()

[tip-bot for Jiri Olsa]

Commit-ID:  705feaf321c37e4dca3637fd5cb3b275f17a06c9
Gitweb:     https://git.kernel.org/tip/705feaf321c37e4dca3637fd5cb3b275f17a06c9
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 12 Mar 2018 14:45:45 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Mar 2018 06:56:08 +0100

hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint()

And rename it to modify_user_hw_breakpoint_check().

We are about to use modify_user_hw_breakpoint_check() for user space
breakpoints modification, we must be very strict to check only the
fields we can change have changed. As Peter explained:

 "Suppose someone does:

        attr = malloc(sizeof(*attr)); // uninitialized memory
        attr->type = BP;
        attr->bp_addr = new_addr;
        attr->bp_type = bp_type;
        attr->bp_len = bp_len;
        ioctl(fd, PERF_IOC_MOD_ATTR, &attr);

  And feeds absolute shite for the rest of the fields.
  Then we later want to extend IOC_MOD_ATTR to allow changing
  attr::sample_type but we can't, because that would break the
  above application."

I'm making this check optional because we already export
modify_user_hw_breakpoint() and with this check we could
break existing users.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Milind Chabbi <chabbi.milind@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20180312134548.31532-6-jolsa@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/hw_breakpoint.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index a556aba223da..0c82663395f7 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -456,7 +456,9 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 }
 EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
 
-static int __modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
+static int
+modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
+			        bool check)
 {
 	u64 old_addr = bp->attr.bp_addr;
 	u64 old_len  = bp->attr.bp_len;
@@ -468,6 +470,9 @@ static int __modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_
 	bp->attr.bp_type = attr->bp_type;
 	bp->attr.bp_len  = attr->bp_len;
 
+	if (check && memcmp(&bp->attr, attr, sizeof(*attr)))
+		return -EINVAL;
+
 	err = validate_hw_breakpoint(bp);
 	if (!err && modify)
 		err = modify_bp_slot(bp, old_type);
@@ -505,7 +510,7 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 	else
 		perf_event_disable(bp);
 
-	err = __modify_user_hw_breakpoint(bp, attr);
+	err = modify_user_hw_breakpoint_check(bp, attr, false);
 
 	if (err) {
 		if (!bp->attr.disabled)

[PATCH 6/8] perf/core: Move perf_event_attr::sample_max_stack into perf_copy_attr

[Jiri Olsa]

Moving sample_max_stack check and setup into perf_copy_attr,
so we have all perf_event_attr initial setup in one place
and can easily compare attrs in the new ioctl introduced
in following change.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/events/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 57898102847f..bd61b4d68a25 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -9834,6 +9834,9 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
 			ret = -EINVAL;
 	}
 
+	if (!attr->sample_max_stack)
+		attr->sample_max_stack = sysctl_perf_event_max_stack;
+
 	if (attr->sample_type & PERF_SAMPLE_REGS_INTR)
 		ret = perf_reg_validate(attr->sample_regs_intr);
 out:
@@ -10047,9 +10050,6 @@ SYSCALL_DEFINE5(perf_event_open,
 	    perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
 		return -EACCES;
 
-	if (!attr.sample_max_stack)
-		attr.sample_max_stack = sysctl_perf_event_max_stack;
-
 	/*
 	 * In cgroup mode, the pid argument is used to pass the fd
 	 * opened to the cgroup directory in cgroupfs. The cpu argument
-- 
2.13.6

[tip:perf/core] perf/core: Move perf_event_attr::sample_max_stack into perf_copy_attr()

[tip-bot for Jiri Olsa]

Commit-ID:  5f970521d3279d99adcdebf329631e36cb9f0deb
Gitweb:     https://git.kernel.org/tip/5f970521d3279d99adcdebf329631e36cb9f0deb
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 12 Mar 2018 14:45:46 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Mar 2018 06:56:08 +0100

perf/core: Move perf_event_attr::sample_max_stack into perf_copy_attr()

Move the sample_max_stack check and setup into perf_copy_attr(),
so we have all perf_event_attr initial setup in one place
and can easily compare attrs in the new ioctl introduced
in following change.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Milind Chabbi <chabbi.milind@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20180312134548.31532-7-jolsa@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 969f865f9f1c..ee145bdee6ed 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10125,6 +10125,9 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
 			ret = -EINVAL;
 	}
 
+	if (!attr->sample_max_stack)
+		attr->sample_max_stack = sysctl_perf_event_max_stack;
+
 	if (attr->sample_type & PERF_SAMPLE_REGS_INTR)
 		ret = perf_reg_validate(attr->sample_regs_intr);
 out:
@@ -10338,9 +10341,6 @@ SYSCALL_DEFINE5(perf_event_open,
 	    perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
 		return -EACCES;
 
-	if (!attr.sample_max_stack)
-		attr.sample_max_stack = sysctl_perf_event_max_stack;
-
 	/*
 	 * In cgroup mode, the pid argument is used to pass the fd
 	 * opened to the cgroup directory in cgroupfs. The cpu argument

[PATCH 7/8] perf/core: fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES.

[Jiri Olsa]

From: Milind Chabbi <chabbi.milind@gmail.com>

Problem and motivation: Once a breakpoint perf event (PERF_TYPE_BREAKPOINT)
is created, there is no flexibility to change the breakpoint type
(bp_type), breakpoint address (bp_addr), or breakpoint length (bp_len). The
only option is to close the perf event and configure a new breakpoint
event. This inflexibility has a significant performance overhead. For
example, sampling-based, lightweight performance profilers (and also
concurrency bug detection tools),  monitor different addresses for a short
duration using PERF_TYPE_BREAKPOINT and change the address (bp_addr) to
another address or change the kind of breakpoint (bp_type) from  "write" to
a "read" or vice-versa or change the length (bp_len) of the address being
monitored. The cost of these modifications is prohibitive since it involves
unmapping the circular buffer associated with the perf event, closing the
perf event, opening another perf event and mmaping another circular buffer.

Solution: The new ioctl flag for perf events,
PERF_EVENT_IOC_MODIFY_ATTRIBUTES, introduced in this patch takes a pointer
to a struct perf_event_attr as an argument to update an old breakpoint
event with new address, type, and size. This facility allows retaining a
previous mmaped perf events ring buffer and avoids having to close and
reopen another perf event.

This patch supports only changing PERF_TYPE_BREAKPOINT event type; future
implementations can extend this feature. The patch replicates some of its
functionality of modify_user_hw_breakpoint() in
kernel/events/hw_breakpoint.c. modify_user_hw_breakpoint cannot be called
directly since perf_event_ctx_lock() is already held in _perf_ioctl().

Evidence: Experiments show that the baseline (not able to modify an already
created breakpoint) costs an order of magnitude (~10x) more than the
suggested optimization (having the ability to dynamically modifying a
configured breakpoint via ioctl). When the breakpoints typically do not
trap, the speedup due to the suggested optimization is ~10x; even when the
breakpoints always trap, the speedup is ~4x due to the suggested
optimization.

Testing: tests posted at
https://github.com/linux-contrib/perf_event_modify_bp demonstrate the
performance significance of this patch. Tests also check the functional
correctness of the patch.

Signed-off-by: Milind Chabbi <chabbi.milind@gmail.com>
[ Using modify_user_hw_breakpoint_check function. ]
[ Reformated PERF_EVENT_IOC_*, so the values are all in one column. ]
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/hw_breakpoint.h         |  7 +++++
 include/uapi/linux/perf_event.h       | 23 +++++++++--------
 kernel/events/core.c                  | 48 +++++++++++++++++++++++++++++++++++
 kernel/events/hw_breakpoint.c         |  2 +-
 tools/include/uapi/linux/perf_event.h | 23 +++++++++--------
 5 files changed, 80 insertions(+), 23 deletions(-)

diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index cf045885a499..abeba094f080 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -53,6 +53,9 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 /* FIXME: only change from the attr, and don't unregister */
 extern int
 modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr);
+extern int
+modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
+				bool check);
 
 /*
  * Kernel breakpoints are not associated with any particular thread.
@@ -97,6 +100,10 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 static inline int
 modify_user_hw_breakpoint(struct perf_event *bp,
 			  struct perf_event_attr *attr)	{ return -ENOSYS; }
+static inline int
+__modify_user_hw_breakpoint(struct perf_event *bp,
+			    struct perf_event_attr *attr) { return -ENOSYS; }
+
 static inline struct perf_event *
 register_wide_hw_breakpoint_cpu(struct perf_event_attr *attr,
 				perf_overflow_handler_t	 triggered,
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 6f873503552d..912b85b52344 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -448,17 +448,18 @@ struct perf_event_query_bpf {
 /*
  * Ioctls that can be done on a perf event fd:
  */
-#define PERF_EVENT_IOC_ENABLE		_IO ('$', 0)
-#define PERF_EVENT_IOC_DISABLE		_IO ('$', 1)
-#define PERF_EVENT_IOC_REFRESH		_IO ('$', 2)
-#define PERF_EVENT_IOC_RESET		_IO ('$', 3)
-#define PERF_EVENT_IOC_PERIOD		_IOW('$', 4, __u64)
-#define PERF_EVENT_IOC_SET_OUTPUT	_IO ('$', 5)
-#define PERF_EVENT_IOC_SET_FILTER	_IOW('$', 6, char *)
-#define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
-#define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
-#define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
-#define PERF_EVENT_IOC_QUERY_BPF	_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_ENABLE			_IO ('$', 0)
+#define PERF_EVENT_IOC_DISABLE			_IO ('$', 1)
+#define PERF_EVENT_IOC_REFRESH			_IO ('$', 2)
+#define PERF_EVENT_IOC_RESET			_IO ('$', 3)
+#define PERF_EVENT_IOC_PERIOD			_IOW('$', 4, __u64)
+#define PERF_EVENT_IOC_SET_OUTPUT		_IO ('$', 5)
+#define PERF_EVENT_IOC_SET_FILTER		_IOW('$', 6, char *)
+#define PERF_EVENT_IOC_ID			_IOR('$', 7, __u64 *)
+#define PERF_EVENT_IOC_SET_BPF			_IOW('$', 8, __u32)
+#define PERF_EVENT_IOC_PAUSE_OUTPUT		_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_QUERY_BPF		_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_MODIFY_ATTRIBUTES	_IOW('$', 11, struct perf_event_attr *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index bd61b4d68a25..7d657f4b1864 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2648,6 +2648,41 @@ int perf_event_refresh(struct perf_event *event, int refresh)
 }
 EXPORT_SYMBOL_GPL(perf_event_refresh);
 
+static int perf_event_modify_breakpoint(struct perf_event *bp,
+					 struct perf_event_attr *attr)
+{
+	int err;
+
+	_perf_event_disable(bp);
+
+	err = modify_user_hw_breakpoint_check(bp, attr, true);
+	if (err) {
+		if (!bp->attr.disabled)
+			_perf_event_enable(bp);
+
+		return err;
+	}
+
+	if (!attr->disabled)
+		_perf_event_enable(bp);
+	return 0;
+}
+
+static int perf_event_modify_attr(struct perf_event *event,
+				  struct perf_event_attr *attr)
+{
+	if (event->attr.type != attr->type)
+		return -EINVAL;
+
+	switch (event->attr.type) {
+	case PERF_TYPE_BREAKPOINT:
+		return perf_event_modify_breakpoint(event, attr);
+	default:
+		/* Place holder for future additions. */
+		return -EOPNOTSUPP;
+	}
+}
+
 static void ctx_sched_out(struct perf_event_context *ctx,
 			  struct perf_cpu_context *cpuctx,
 			  enum event_type_t event_type)
@@ -4663,6 +4698,8 @@ static int perf_event_set_output(struct perf_event *event,
 				 struct perf_event *output_event);
 static int perf_event_set_filter(struct perf_event *event, void __user *arg);
 static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd);
+static int perf_copy_attr(struct perf_event_attr __user *uattr,
+			  struct perf_event_attr *attr);
 
 static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned long arg)
 {
@@ -4735,6 +4772,17 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
 
 	case PERF_EVENT_IOC_QUERY_BPF:
 		return perf_event_query_prog_array(event, (void __user *)arg);
+
+	case PERF_EVENT_IOC_MODIFY_ATTRIBUTES: {
+		struct perf_event_attr new_attr;
+		int err = perf_copy_attr((struct perf_event_attr __user *)arg,
+					 &new_attr);
+
+		if (err)
+			return err;
+
+		return perf_event_modify_attr(event,  &new_attr);
+	}
 	default:
 		return -ENOTTY;
 	}
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 0c82663395f7..6253d5519cd8 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -456,7 +456,7 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 }
 EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
 
-static int
+int
 modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
 			        bool check)
 {
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index 6f873503552d..912b85b52344 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -448,17 +448,18 @@ struct perf_event_query_bpf {
 /*
  * Ioctls that can be done on a perf event fd:
  */
-#define PERF_EVENT_IOC_ENABLE		_IO ('$', 0)
-#define PERF_EVENT_IOC_DISABLE		_IO ('$', 1)
-#define PERF_EVENT_IOC_REFRESH		_IO ('$', 2)
-#define PERF_EVENT_IOC_RESET		_IO ('$', 3)
-#define PERF_EVENT_IOC_PERIOD		_IOW('$', 4, __u64)
-#define PERF_EVENT_IOC_SET_OUTPUT	_IO ('$', 5)
-#define PERF_EVENT_IOC_SET_FILTER	_IOW('$', 6, char *)
-#define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
-#define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
-#define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
-#define PERF_EVENT_IOC_QUERY_BPF	_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_ENABLE			_IO ('$', 0)
+#define PERF_EVENT_IOC_DISABLE			_IO ('$', 1)
+#define PERF_EVENT_IOC_REFRESH			_IO ('$', 2)
+#define PERF_EVENT_IOC_RESET			_IO ('$', 3)
+#define PERF_EVENT_IOC_PERIOD			_IOW('$', 4, __u64)
+#define PERF_EVENT_IOC_SET_OUTPUT		_IO ('$', 5)
+#define PERF_EVENT_IOC_SET_FILTER		_IOW('$', 6, char *)
+#define PERF_EVENT_IOC_ID			_IOR('$', 7, __u64 *)
+#define PERF_EVENT_IOC_SET_BPF			_IOW('$', 8, __u32)
+#define PERF_EVENT_IOC_PAUSE_OUTPUT		_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_QUERY_BPF		_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_MODIFY_ATTRIBUTES	_IOW('$', 11, struct perf_event_attr *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,
-- 
2.13.6

[tip:perf/core] perf/core: Implement fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES

[tip-bot for Milind Chabbi]

Commit-ID:  f30b09b7f8aed3180d6e2f2984e32e91c7a7fcd1
Gitweb:     https://git.kernel.org/tip/f30b09b7f8aed3180d6e2f2984e32e91c7a7fcd1
Author:     Milind Chabbi <chabbi.milind@gmail.com>
AuthorDate: Mon, 12 Mar 2018 14:45:47 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Mar 2018 06:56:08 +0100

perf/core: Implement fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES

Problem and motivation: Once a breakpoint perf event (PERF_TYPE_BREAKPOINT)
is created, there is no flexibility to change the breakpoint type
(bp_type), breakpoint address (bp_addr), or breakpoint length (bp_len). The
only option is to close the perf event and configure a new breakpoint
event. This inflexibility has a significant performance overhead. For
example, sampling-based, lightweight performance profilers (and also
concurrency bug detection tools),  monitor different addresses for a short
duration using PERF_TYPE_BREAKPOINT and change the address (bp_addr) to
another address or change the kind of breakpoint (bp_type) from  "write" to
a "read" or vice-versa or change the length (bp_len) of the address being
monitored. The cost of these modifications is prohibitive since it involves
unmapping the circular buffer associated with the perf event, closing the
perf event, opening another perf event and mmaping another circular buffer.

Solution: The new ioctl flag for perf events,
PERF_EVENT_IOC_MODIFY_ATTRIBUTES, introduced in this patch takes a pointer
to a struct perf_event_attr as an argument to update an old breakpoint
event with new address, type, and size. This facility allows retaining a
previous mmaped perf events ring buffer and avoids having to close and
reopen another perf event.

This patch supports only changing PERF_TYPE_BREAKPOINT event type; future
implementations can extend this feature. The patch replicates some of its
functionality of modify_user_hw_breakpoint() in
kernel/events/hw_breakpoint.c. modify_user_hw_breakpoint cannot be called
directly since perf_event_ctx_lock() is already held in _perf_ioctl().

Evidence: Experiments show that the baseline (not able to modify an already
created breakpoint) costs an order of magnitude (~10x) more than the
suggested optimization (having the ability to dynamically modifying a
configured breakpoint via ioctl). When the breakpoints typically do not
trap, the speedup due to the suggested optimization is ~10x; even when the
breakpoints always trap, the speedup is ~4x due to the suggested
optimization.

Testing: tests posted at
https://github.com/linux-contrib/perf_event_modify_bp demonstrate the
performance significance of this patch. Tests also check the functional
correctness of the patch.

Signed-off-by: Milind Chabbi <chabbi.milind@gmail.com>
[ Using modify_user_hw_breakpoint_check function. ]
[ Reformated PERF_EVENT_IOC_*, so the values are all in one column. ]
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20180312134548.31532-8-jolsa@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/hw_breakpoint.h         |  7 +++++
 include/uapi/linux/perf_event.h       | 23 +++++++++--------
 kernel/events/core.c                  | 48 +++++++++++++++++++++++++++++++++++
 kernel/events/hw_breakpoint.c         |  2 +-
 tools/include/uapi/linux/perf_event.h | 23 +++++++++--------
 5 files changed, 80 insertions(+), 23 deletions(-)

diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index cf045885a499..abeba094f080 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -53,6 +53,9 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 /* FIXME: only change from the attr, and don't unregister */
 extern int
 modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr);
+extern int
+modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
+				bool check);
 
 /*
  * Kernel breakpoints are not associated with any particular thread.
@@ -97,6 +100,10 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 static inline int
 modify_user_hw_breakpoint(struct perf_event *bp,
 			  struct perf_event_attr *attr)	{ return -ENOSYS; }
+static inline int
+__modify_user_hw_breakpoint(struct perf_event *bp,
+			    struct perf_event_attr *attr) { return -ENOSYS; }
+
 static inline struct perf_event *
 register_wide_hw_breakpoint_cpu(struct perf_event_attr *attr,
 				perf_overflow_handler_t	 triggered,
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 6f873503552d..912b85b52344 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -448,17 +448,18 @@ struct perf_event_query_bpf {
 /*
  * Ioctls that can be done on a perf event fd:
  */
-#define PERF_EVENT_IOC_ENABLE		_IO ('$', 0)
-#define PERF_EVENT_IOC_DISABLE		_IO ('$', 1)
-#define PERF_EVENT_IOC_REFRESH		_IO ('$', 2)
-#define PERF_EVENT_IOC_RESET		_IO ('$', 3)
-#define PERF_EVENT_IOC_PERIOD		_IOW('$', 4, __u64)
-#define PERF_EVENT_IOC_SET_OUTPUT	_IO ('$', 5)
-#define PERF_EVENT_IOC_SET_FILTER	_IOW('$', 6, char *)
-#define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
-#define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
-#define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
-#define PERF_EVENT_IOC_QUERY_BPF	_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_ENABLE			_IO ('$', 0)
+#define PERF_EVENT_IOC_DISABLE			_IO ('$', 1)
+#define PERF_EVENT_IOC_REFRESH			_IO ('$', 2)
+#define PERF_EVENT_IOC_RESET			_IO ('$', 3)
+#define PERF_EVENT_IOC_PERIOD			_IOW('$', 4, __u64)
+#define PERF_EVENT_IOC_SET_OUTPUT		_IO ('$', 5)
+#define PERF_EVENT_IOC_SET_FILTER		_IOW('$', 6, char *)
+#define PERF_EVENT_IOC_ID			_IOR('$', 7, __u64 *)
+#define PERF_EVENT_IOC_SET_BPF			_IOW('$', 8, __u32)
+#define PERF_EVENT_IOC_PAUSE_OUTPUT		_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_QUERY_BPF		_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_MODIFY_ATTRIBUTES	_IOW('$', 11, struct perf_event_attr *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index ee145bdee6ed..3b4c7792a6ac 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2846,6 +2846,41 @@ int perf_event_refresh(struct perf_event *event, int refresh)
 }
 EXPORT_SYMBOL_GPL(perf_event_refresh);
 
+static int perf_event_modify_breakpoint(struct perf_event *bp,
+					 struct perf_event_attr *attr)
+{
+	int err;
+
+	_perf_event_disable(bp);
+
+	err = modify_user_hw_breakpoint_check(bp, attr, true);
+	if (err) {
+		if (!bp->attr.disabled)
+			_perf_event_enable(bp);
+
+		return err;
+	}
+
+	if (!attr->disabled)
+		_perf_event_enable(bp);
+	return 0;
+}
+
+static int perf_event_modify_attr(struct perf_event *event,
+				  struct perf_event_attr *attr)
+{
+	if (event->attr.type != attr->type)
+		return -EINVAL;
+
+	switch (event->attr.type) {
+	case PERF_TYPE_BREAKPOINT:
+		return perf_event_modify_breakpoint(event, attr);
+	default:
+		/* Place holder for future additions. */
+		return -EOPNOTSUPP;
+	}
+}
+
 static void ctx_sched_out(struct perf_event_context *ctx,
 			  struct perf_cpu_context *cpuctx,
 			  enum event_type_t event_type)
@@ -4952,6 +4987,8 @@ static int perf_event_set_output(struct perf_event *event,
 				 struct perf_event *output_event);
 static int perf_event_set_filter(struct perf_event *event, void __user *arg);
 static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd);
+static int perf_copy_attr(struct perf_event_attr __user *uattr,
+			  struct perf_event_attr *attr);
 
 static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned long arg)
 {
@@ -5024,6 +5061,17 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
 
 	case PERF_EVENT_IOC_QUERY_BPF:
 		return perf_event_query_prog_array(event, (void __user *)arg);
+
+	case PERF_EVENT_IOC_MODIFY_ATTRIBUTES: {
+		struct perf_event_attr new_attr;
+		int err = perf_copy_attr((struct perf_event_attr __user *)arg,
+					 &new_attr);
+
+		if (err)
+			return err;
+
+		return perf_event_modify_attr(event,  &new_attr);
+	}
 	default:
 		return -ENOTTY;
 	}
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 0c82663395f7..6253d5519cd8 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -456,7 +456,7 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 }
 EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
 
-static int
+int
 modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
 			        bool check)
 {
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index 6f873503552d..912b85b52344 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -448,17 +448,18 @@ struct perf_event_query_bpf {
 /*
  * Ioctls that can be done on a perf event fd:
  */
-#define PERF_EVENT_IOC_ENABLE		_IO ('$', 0)
-#define PERF_EVENT_IOC_DISABLE		_IO ('$', 1)
-#define PERF_EVENT_IOC_REFRESH		_IO ('$', 2)
-#define PERF_EVENT_IOC_RESET		_IO ('$', 3)
-#define PERF_EVENT_IOC_PERIOD		_IOW('$', 4, __u64)
-#define PERF_EVENT_IOC_SET_OUTPUT	_IO ('$', 5)
-#define PERF_EVENT_IOC_SET_FILTER	_IOW('$', 6, char *)
-#define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
-#define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
-#define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
-#define PERF_EVENT_IOC_QUERY_BPF	_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_ENABLE			_IO ('$', 0)
+#define PERF_EVENT_IOC_DISABLE			_IO ('$', 1)
+#define PERF_EVENT_IOC_REFRESH			_IO ('$', 2)
+#define PERF_EVENT_IOC_RESET			_IO ('$', 3)
+#define PERF_EVENT_IOC_PERIOD			_IOW('$', 4, __u64)
+#define PERF_EVENT_IOC_SET_OUTPUT		_IO ('$', 5)
+#define PERF_EVENT_IOC_SET_FILTER		_IOW('$', 6, char *)
+#define PERF_EVENT_IOC_ID			_IOR('$', 7, __u64 *)
+#define PERF_EVENT_IOC_SET_BPF			_IOW('$', 8, __u32)
+#define PERF_EVENT_IOC_PAUSE_OUTPUT		_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_QUERY_BPF		_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_MODIFY_ATTRIBUTES	_IOW('$', 11, struct perf_event_attr *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,

[tip:perf/core] perf/core: Implement fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES

[tip-bot for Milind Chabbi]

Commit-ID:  32ff77e8cc9e66cc4fb38098f64fd54cc8f54573
Gitweb:     https://git.kernel.org/tip/32ff77e8cc9e66cc4fb38098f64fd54cc8f54573
Author:     Milind Chabbi <chabbi.milind@gmail.com>
AuthorDate: Mon, 12 Mar 2018 14:45:47 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Mar 2018 15:24:02 +0100

perf/core: Implement fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES

Problem and motivation: Once a breakpoint perf event (PERF_TYPE_BREAKPOINT)
is created, there is no flexibility to change the breakpoint type
(bp_type), breakpoint address (bp_addr), or breakpoint length (bp_len). The
only option is to close the perf event and configure a new breakpoint
event. This inflexibility has a significant performance overhead. For
example, sampling-based, lightweight performance profilers (and also
concurrency bug detection tools),  monitor different addresses for a short
duration using PERF_TYPE_BREAKPOINT and change the address (bp_addr) to
another address or change the kind of breakpoint (bp_type) from  "write" to
a "read" or vice-versa or change the length (bp_len) of the address being
monitored. The cost of these modifications is prohibitive since it involves
unmapping the circular buffer associated with the perf event, closing the
perf event, opening another perf event and mmaping another circular buffer.

Solution: The new ioctl flag for perf events,
PERF_EVENT_IOC_MODIFY_ATTRIBUTES, introduced in this patch takes a pointer
to a struct perf_event_attr as an argument to update an old breakpoint
event with new address, type, and size. This facility allows retaining a
previous mmaped perf events ring buffer and avoids having to close and
reopen another perf event.

This patch supports only changing PERF_TYPE_BREAKPOINT event type; future
implementations can extend this feature. The patch replicates some of its
functionality of modify_user_hw_breakpoint() in
kernel/events/hw_breakpoint.c. modify_user_hw_breakpoint cannot be called
directly since perf_event_ctx_lock() is already held in _perf_ioctl().

Evidence: Experiments show that the baseline (not able to modify an already
created breakpoint) costs an order of magnitude (~10x) more than the
suggested optimization (having the ability to dynamically modifying a
configured breakpoint via ioctl). When the breakpoints typically do not
trap, the speedup due to the suggested optimization is ~10x; even when the
breakpoints always trap, the speedup is ~4x due to the suggested
optimization.

Testing: tests posted at
https://github.com/linux-contrib/perf_event_modify_bp demonstrate the
performance significance of this patch. Tests also check the functional
correctness of the patch.

Signed-off-by: Milind Chabbi <chabbi.milind@gmail.com>
[ Using modify_user_hw_breakpoint_check function. ]
[ Reformated PERF_EVENT_IOC_*, so the values are all in one column. ]
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20180312134548.31532-8-jolsa@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/hw_breakpoint.h         |  7 +++++
 include/uapi/linux/perf_event.h       | 23 +++++++++--------
 kernel/events/core.c                  | 48 +++++++++++++++++++++++++++++++++++
 kernel/events/hw_breakpoint.c         |  2 +-
 tools/include/uapi/linux/perf_event.h | 23 +++++++++--------
 5 files changed, 80 insertions(+), 23 deletions(-)

diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index cf045885a499..6058c3844a76 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -53,6 +53,9 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 /* FIXME: only change from the attr, and don't unregister */
 extern int
 modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr);
+extern int
+modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
+				bool check);
 
 /*
  * Kernel breakpoints are not associated with any particular thread.
@@ -97,6 +100,10 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 static inline int
 modify_user_hw_breakpoint(struct perf_event *bp,
 			  struct perf_event_attr *attr)	{ return -ENOSYS; }
+static inline int
+modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
+				bool check)	{ return -ENOSYS; }
+
 static inline struct perf_event *
 register_wide_hw_breakpoint_cpu(struct perf_event_attr *attr,
 				perf_overflow_handler_t	 triggered,
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 6f873503552d..912b85b52344 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -448,17 +448,18 @@ struct perf_event_query_bpf {
 /*
  * Ioctls that can be done on a perf event fd:
  */
-#define PERF_EVENT_IOC_ENABLE		_IO ('$', 0)
-#define PERF_EVENT_IOC_DISABLE		_IO ('$', 1)
-#define PERF_EVENT_IOC_REFRESH		_IO ('$', 2)
-#define PERF_EVENT_IOC_RESET		_IO ('$', 3)
-#define PERF_EVENT_IOC_PERIOD		_IOW('$', 4, __u64)
-#define PERF_EVENT_IOC_SET_OUTPUT	_IO ('$', 5)
-#define PERF_EVENT_IOC_SET_FILTER	_IOW('$', 6, char *)
-#define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
-#define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
-#define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
-#define PERF_EVENT_IOC_QUERY_BPF	_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_ENABLE			_IO ('$', 0)
+#define PERF_EVENT_IOC_DISABLE			_IO ('$', 1)
+#define PERF_EVENT_IOC_REFRESH			_IO ('$', 2)
+#define PERF_EVENT_IOC_RESET			_IO ('$', 3)
+#define PERF_EVENT_IOC_PERIOD			_IOW('$', 4, __u64)
+#define PERF_EVENT_IOC_SET_OUTPUT		_IO ('$', 5)
+#define PERF_EVENT_IOC_SET_FILTER		_IOW('$', 6, char *)
+#define PERF_EVENT_IOC_ID			_IOR('$', 7, __u64 *)
+#define PERF_EVENT_IOC_SET_BPF			_IOW('$', 8, __u32)
+#define PERF_EVENT_IOC_PAUSE_OUTPUT		_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_QUERY_BPF		_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_MODIFY_ATTRIBUTES	_IOW('$', 11, struct perf_event_attr *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index ee145bdee6ed..3b4c7792a6ac 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2846,6 +2846,41 @@ int perf_event_refresh(struct perf_event *event, int refresh)
 }
 EXPORT_SYMBOL_GPL(perf_event_refresh);
 
+static int perf_event_modify_breakpoint(struct perf_event *bp,
+					 struct perf_event_attr *attr)
+{
+	int err;
+
+	_perf_event_disable(bp);
+
+	err = modify_user_hw_breakpoint_check(bp, attr, true);
+	if (err) {
+		if (!bp->attr.disabled)
+			_perf_event_enable(bp);
+
+		return err;
+	}
+
+	if (!attr->disabled)
+		_perf_event_enable(bp);
+	return 0;
+}
+
+static int perf_event_modify_attr(struct perf_event *event,
+				  struct perf_event_attr *attr)
+{
+	if (event->attr.type != attr->type)
+		return -EINVAL;
+
+	switch (event->attr.type) {
+	case PERF_TYPE_BREAKPOINT:
+		return perf_event_modify_breakpoint(event, attr);
+	default:
+		/* Place holder for future additions. */
+		return -EOPNOTSUPP;
+	}
+}
+
 static void ctx_sched_out(struct perf_event_context *ctx,
 			  struct perf_cpu_context *cpuctx,
 			  enum event_type_t event_type)
@@ -4952,6 +4987,8 @@ static int perf_event_set_output(struct perf_event *event,
 				 struct perf_event *output_event);
 static int perf_event_set_filter(struct perf_event *event, void __user *arg);
 static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd);
+static int perf_copy_attr(struct perf_event_attr __user *uattr,
+			  struct perf_event_attr *attr);
 
 static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned long arg)
 {
@@ -5024,6 +5061,17 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
 
 	case PERF_EVENT_IOC_QUERY_BPF:
 		return perf_event_query_prog_array(event, (void __user *)arg);
+
+	case PERF_EVENT_IOC_MODIFY_ATTRIBUTES: {
+		struct perf_event_attr new_attr;
+		int err = perf_copy_attr((struct perf_event_attr __user *)arg,
+					 &new_attr);
+
+		if (err)
+			return err;
+
+		return perf_event_modify_attr(event,  &new_attr);
+	}
 	default:
 		return -ENOTTY;
 	}
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 0c82663395f7..6253d5519cd8 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -456,7 +456,7 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 }
 EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
 
-static int
+int
 modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
 			        bool check)
 {
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index 6f873503552d..912b85b52344 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -448,17 +448,18 @@ struct perf_event_query_bpf {
 /*
  * Ioctls that can be done on a perf event fd:
  */
-#define PERF_EVENT_IOC_ENABLE		_IO ('$', 0)
-#define PERF_EVENT_IOC_DISABLE		_IO ('$', 1)
-#define PERF_EVENT_IOC_REFRESH		_IO ('$', 2)
-#define PERF_EVENT_IOC_RESET		_IO ('$', 3)
-#define PERF_EVENT_IOC_PERIOD		_IOW('$', 4, __u64)
-#define PERF_EVENT_IOC_SET_OUTPUT	_IO ('$', 5)
-#define PERF_EVENT_IOC_SET_FILTER	_IOW('$', 6, char *)
-#define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
-#define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
-#define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
-#define PERF_EVENT_IOC_QUERY_BPF	_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_ENABLE			_IO ('$', 0)
+#define PERF_EVENT_IOC_DISABLE			_IO ('$', 1)
+#define PERF_EVENT_IOC_REFRESH			_IO ('$', 2)
+#define PERF_EVENT_IOC_RESET			_IO ('$', 3)
+#define PERF_EVENT_IOC_PERIOD			_IOW('$', 4, __u64)
+#define PERF_EVENT_IOC_SET_OUTPUT		_IO ('$', 5)
+#define PERF_EVENT_IOC_SET_FILTER		_IOW('$', 6, char *)
+#define PERF_EVENT_IOC_ID			_IOR('$', 7, __u64 *)
+#define PERF_EVENT_IOC_SET_BPF			_IOW('$', 8, __u32)
+#define PERF_EVENT_IOC_PAUSE_OUTPUT		_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_QUERY_BPF		_IOWR('$', 10, struct perf_event_query_bpf *)
+#define PERF_EVENT_IOC_MODIFY_ATTRIBUTES	_IOW('$', 11, struct perf_event_attr *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,

[PATCH 8/8] perf tests: Add breakpoint accounting/modify test

[Jiri Olsa]

Adding test that:
  - detects the number of watch/break-points,
    skip test if any is missing
  - detects PERF_EVENT_IOC_MODIFY_ATTRIBUTES ioctl,
    skip test if it's missing
  - detects if watchpoints and breakpoints share
    same slots
  - create all possible watchpoints on cpu 0
  - change one of it to breakpoint
  - in case wp and bp do not share slots,
    we create another watchpoint to ensure
    the slot accounting is correct

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/Build          |   1 +
 tools/perf/tests/bp_account.c   | 195 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/builtin-test.c |   4 +
 tools/perf/tests/tests.h        |   1 +
 4 files changed, 201 insertions(+)
 create mode 100644 tools/perf/tests/bp_account.c

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 45782220ac23..6c108fa79ae3 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -20,6 +20,7 @@ perf-y += hists_cumulate.o
 perf-y += python-use.o
 perf-y += bp_signal.o
 perf-y += bp_signal_overflow.o
+perf-y += bp_account.o
 perf-y += task-exit.o
 perf-y += sw-clock.o
 perf-y += mmap-thread-lookup.o
diff --git a/tools/perf/tests/bp_account.c b/tools/perf/tests/bp_account.c
new file mode 100644
index 000000000000..2f75fa0c4fef
--- /dev/null
+++ b/tools/perf/tests/bp_account.c
@@ -0,0 +1,195 @@
+/*
+ * Powerpc needs __SANE_USERSPACE_TYPES__ before <linux/types.h> to select
+ * 'int-ll64.h' and avoid compile warnings when printing __u64 with %llu.
+ */
+#define __SANE_USERSPACE_TYPES__
+
+#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 <sys/ioctl.h>
+
+#include "tests.h"
+#include "debug.h"
+#include "perf.h"
+#include "cloexec.h"
+
+volatile long the_var;
+
+static noinline int test_function(void)
+{
+	return 0;
+}
+
+static int __event(bool is_x, void *addr, struct perf_event_attr *attr)
+{
+	int fd;
+
+	memset(attr, 0, sizeof(struct perf_event_attr));
+	attr->type = PERF_TYPE_BREAKPOINT;
+	attr->size = sizeof(struct perf_event_attr);
+
+	attr->config = 0;
+	attr->bp_type = is_x ? HW_BREAKPOINT_X : HW_BREAKPOINT_W;
+	attr->bp_addr = (unsigned long) addr;
+	attr->bp_len = sizeof(long);
+
+	attr->sample_period = 1;
+	attr->sample_type = PERF_SAMPLE_IP;
+
+	attr->exclude_kernel = 1;
+	attr->exclude_hv = 1;
+
+	fd = sys_perf_event_open(attr, -1, 0, -1,
+				 perf_event_open_cloexec_flag());
+	if (fd < 0) {
+		pr_debug("failed opening event %llx\n", attr->config);
+		return TEST_FAIL;
+	}
+
+	return fd;
+}
+
+static int wp_event(void *addr, struct perf_event_attr *attr)
+{
+	return __event(false, addr, attr);
+}
+
+static int bp_event(void *addr, struct perf_event_attr *attr)
+{
+	return __event(true, addr, attr);
+}
+
+static int bp_accounting(int wp_cnt, int share)
+{
+	struct perf_event_attr attr, attr_mod, attr_new;
+	int i, fd[wp_cnt], fd_wp, ret;
+
+	for (i = 0; i < wp_cnt; i++) {
+		fd[i] = wp_event((void *)&the_var, &attr);
+		TEST_ASSERT_VAL("failed to create wp\n", fd[i] != -1);
+		pr_debug("wp %d created\n", i);
+	}
+
+	attr_mod = attr;
+	attr_mod.bp_type = HW_BREAKPOINT_X;
+	attr_mod.bp_addr = (unsigned long) test_function;
+
+	ret = ioctl(fd[0], PERF_EVENT_IOC_MODIFY_ATTRIBUTES, &attr_mod);
+	TEST_ASSERT_VAL("failed to modify wp\n", ret == 0);
+
+	pr_debug("wp 0 modified to bp\n");
+
+	if (!share) {
+		fd_wp = wp_event((void *)&the_var, &attr_new);
+		TEST_ASSERT_VAL("failed to create max wp\n", fd_wp != -1);
+		pr_debug("wp max created\n");
+	}
+
+	for (i = 0; i < wp_cnt; i++)
+		close(fd[i]);
+
+	return 0;
+}
+
+static int detect_cnt(bool is_x)
+{
+	struct perf_event_attr attr;
+	void *addr = is_x ? test_function : (void *) &the_var;
+	int fd[100], cnt = 0, i;
+
+	while (1) {
+		fd[cnt] = __event(is_x, addr, &attr);
+
+		if (fd[cnt] < 0)
+			break;
+
+		if (cnt == 100) {
+			pr_debug("way too many debug registers, fix the test\n");
+			return 0;
+		}
+
+		cnt++;
+	}
+
+	for (i = 0; i < cnt; i++)
+		close(fd[i]);
+
+	return cnt;
+}
+
+static int detect_ioctl(void)
+{
+	struct perf_event_attr attr;
+	int fd, ret = 1;
+
+	fd = wp_event((void *) &the_var, &attr);
+	if (fd > 0) {
+		ret = ioctl(fd, PERF_EVENT_IOC_MODIFY_ATTRIBUTES, &attr);
+		close(fd);
+	}
+
+	return ret ? 0 : 1;
+}
+
+static int detect_share(int wp_cnt, int bp_cnt)
+{
+	struct perf_event_attr attr;
+	int i, fd[wp_cnt + bp_cnt], ret;
+
+	for (i = 0; i < wp_cnt; i++) {
+		fd[i] = wp_event((void *)&the_var, &attr);
+		TEST_ASSERT_VAL("failed to create wp\n", fd[i] != -1);
+	}
+
+	for (; i < (bp_cnt + wp_cnt); i++) {
+		fd[i] = bp_event((void *)test_function, &attr);
+		if (fd[i] == -1)
+			break;
+	}
+
+	ret = i != (bp_cnt + wp_cnt);
+
+	while (i--)
+		close(fd[i]);
+
+	return ret;
+}
+
+/*
+ * This test does following:
+ *   - detects the number of watch/break-points,
+ *     skip test if any is missing
+ *   - detects PERF_EVENT_IOC_MODIFY_ATTRIBUTES ioctl,
+ *     skip test if it's missing
+ *   - detects if watchpoints and breakpoints share
+ *     same slots
+ *   - create all possible watchpoints on cpu 0
+ *   - change one of it to breakpoint
+ *   - in case wp and bp do not share slots,
+ *     we create another watchpoint to ensure
+ *     the slot accounting is correct
+ */
+int test__bp_accounting(struct test *test __maybe_unused, int subtest __maybe_unused)
+{
+	int has_ioctl = detect_ioctl();
+	int wp_cnt = detect_cnt(false);
+	int bp_cnt = detect_cnt(true);
+	int share  = detect_share(wp_cnt, bp_cnt);
+
+	pr_debug("watchpoints count %d, breakpoints count %d, has_ioctl %d, share %d\n",
+		 wp_cnt, bp_cnt, has_ioctl, share);
+
+	if (!wp_cnt || !bp_cnt || !has_ioctl)
+		return TEST_SKIP;
+
+	return bp_accounting(wp_cnt, share);
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 09071ef4434f..625f5a6772af 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -116,6 +116,10 @@ static struct test generic_tests[] = {
 		.is_supported = test__bp_signal_is_supported,
 	},
 	{
+		.desc = "Breakpoint accounting",
+		.func = test__bp_accounting,
+	},
+	{
 		.desc = "Number of exit events of a simple workload",
 		.func = test__task_exit,
 	},
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 2e169819e647..a9760e790563 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -58,6 +58,7 @@ int test__hists_link(struct test *test, int subtest);
 int test__python_use(struct test *test, int subtest);
 int test__bp_signal(struct test *test, int subtest);
 int test__bp_signal_overflow(struct test *test, int subtest);
+int test__bp_accounting(struct test *test, int subtest);
 int test__task_exit(struct test *test, int subtest);
 int test__mem(struct test *test, int subtest);
 int test__sw_clock_freq(struct test *test, int subtest);
-- 
2.13.6

[tip:perf/core] perf tests: Add breakpoint accounting/modify test

[tip-bot for Jiri Olsa]

Commit-ID:  5acc575fec9f424f0b557844dbda8aed57baae1e
Gitweb:     https://git.kernel.org/tip/5acc575fec9f424f0b557844dbda8aed57baae1e
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 12 Mar 2018 14:45:48 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Mar 2018 06:56:09 +0100

perf tests: Add breakpoint accounting/modify test

Adding test that:

  - detects the number of watch/break-points,
    skip test if any is missing
  - detects PERF_EVENT_IOC_MODIFY_ATTRIBUTES ioctl,
    skip test if it's missing
  - detects if watchpoints and breakpoints share
    same slots
  - create all possible watchpoints on cpu 0
  - change one of it to breakpoint
  - in case wp and bp do not share slots,
    we create another watchpoint to ensure
    the slot accounting is correct

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Milind Chabbi <chabbi.milind@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20180312134548.31532-9-jolsa@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/tests/Build          |   1 +
 tools/perf/tests/bp_account.c   | 195 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/builtin-test.c |   4 +
 tools/perf/tests/tests.h        |   1 +
 4 files changed, 201 insertions(+)

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 87bf3edb037c..62ca0174d5e1 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -20,6 +20,7 @@ perf-y += hists_cumulate.o
 perf-y += python-use.o
 perf-y += bp_signal.o
 perf-y += bp_signal_overflow.o
+perf-y += bp_account.o
 perf-y += task-exit.o
 perf-y += sw-clock.o
 perf-y += mmap-thread-lookup.o
diff --git a/tools/perf/tests/bp_account.c b/tools/perf/tests/bp_account.c
new file mode 100644
index 000000000000..2f75fa0c4fef
--- /dev/null
+++ b/tools/perf/tests/bp_account.c
@@ -0,0 +1,195 @@
+/*
+ * Powerpc needs __SANE_USERSPACE_TYPES__ before <linux/types.h> to select
+ * 'int-ll64.h' and avoid compile warnings when printing __u64 with %llu.
+ */
+#define __SANE_USERSPACE_TYPES__
+
+#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 <sys/ioctl.h>
+
+#include "tests.h"
+#include "debug.h"
+#include "perf.h"
+#include "cloexec.h"
+
+volatile long the_var;
+
+static noinline int test_function(void)
+{
+	return 0;
+}
+
+static int __event(bool is_x, void *addr, struct perf_event_attr *attr)
+{
+	int fd;
+
+	memset(attr, 0, sizeof(struct perf_event_attr));
+	attr->type = PERF_TYPE_BREAKPOINT;
+	attr->size = sizeof(struct perf_event_attr);
+
+	attr->config = 0;
+	attr->bp_type = is_x ? HW_BREAKPOINT_X : HW_BREAKPOINT_W;
+	attr->bp_addr = (unsigned long) addr;
+	attr->bp_len = sizeof(long);
+
+	attr->sample_period = 1;
+	attr->sample_type = PERF_SAMPLE_IP;
+
+	attr->exclude_kernel = 1;
+	attr->exclude_hv = 1;
+
+	fd = sys_perf_event_open(attr, -1, 0, -1,
+				 perf_event_open_cloexec_flag());
+	if (fd < 0) {
+		pr_debug("failed opening event %llx\n", attr->config);
+		return TEST_FAIL;
+	}
+
+	return fd;
+}
+
+static int wp_event(void *addr, struct perf_event_attr *attr)
+{
+	return __event(false, addr, attr);
+}
+
+static int bp_event(void *addr, struct perf_event_attr *attr)
+{
+	return __event(true, addr, attr);
+}
+
+static int bp_accounting(int wp_cnt, int share)
+{
+	struct perf_event_attr attr, attr_mod, attr_new;
+	int i, fd[wp_cnt], fd_wp, ret;
+
+	for (i = 0; i < wp_cnt; i++) {
+		fd[i] = wp_event((void *)&the_var, &attr);
+		TEST_ASSERT_VAL("failed to create wp\n", fd[i] != -1);
+		pr_debug("wp %d created\n", i);
+	}
+
+	attr_mod = attr;
+	attr_mod.bp_type = HW_BREAKPOINT_X;
+	attr_mod.bp_addr = (unsigned long) test_function;
+
+	ret = ioctl(fd[0], PERF_EVENT_IOC_MODIFY_ATTRIBUTES, &attr_mod);
+	TEST_ASSERT_VAL("failed to modify wp\n", ret == 0);
+
+	pr_debug("wp 0 modified to bp\n");
+
+	if (!share) {
+		fd_wp = wp_event((void *)&the_var, &attr_new);
+		TEST_ASSERT_VAL("failed to create max wp\n", fd_wp != -1);
+		pr_debug("wp max created\n");
+	}
+
+	for (i = 0; i < wp_cnt; i++)
+		close(fd[i]);
+
+	return 0;
+}
+
+static int detect_cnt(bool is_x)
+{
+	struct perf_event_attr attr;
+	void *addr = is_x ? test_function : (void *) &the_var;
+	int fd[100], cnt = 0, i;
+
+	while (1) {
+		fd[cnt] = __event(is_x, addr, &attr);
+
+		if (fd[cnt] < 0)
+			break;
+
+		if (cnt == 100) {
+			pr_debug("way too many debug registers, fix the test\n");
+			return 0;
+		}
+
+		cnt++;
+	}
+
+	for (i = 0; i < cnt; i++)
+		close(fd[i]);
+
+	return cnt;
+}
+
+static int detect_ioctl(void)
+{
+	struct perf_event_attr attr;
+	int fd, ret = 1;
+
+	fd = wp_event((void *) &the_var, &attr);
+	if (fd > 0) {
+		ret = ioctl(fd, PERF_EVENT_IOC_MODIFY_ATTRIBUTES, &attr);
+		close(fd);
+	}
+
+	return ret ? 0 : 1;
+}
+
+static int detect_share(int wp_cnt, int bp_cnt)
+{
+	struct perf_event_attr attr;
+	int i, fd[wp_cnt + bp_cnt], ret;
+
+	for (i = 0; i < wp_cnt; i++) {
+		fd[i] = wp_event((void *)&the_var, &attr);
+		TEST_ASSERT_VAL("failed to create wp\n", fd[i] != -1);
+	}
+
+	for (; i < (bp_cnt + wp_cnt); i++) {
+		fd[i] = bp_event((void *)test_function, &attr);
+		if (fd[i] == -1)
+			break;
+	}
+
+	ret = i != (bp_cnt + wp_cnt);
+
+	while (i--)
+		close(fd[i]);
+
+	return ret;
+}
+
+/*
+ * This test does following:
+ *   - detects the number of watch/break-points,
+ *     skip test if any is missing
+ *   - detects PERF_EVENT_IOC_MODIFY_ATTRIBUTES ioctl,
+ *     skip test if it's missing
+ *   - detects if watchpoints and breakpoints share
+ *     same slots
+ *   - create all possible watchpoints on cpu 0
+ *   - change one of it to breakpoint
+ *   - in case wp and bp do not share slots,
+ *     we create another watchpoint to ensure
+ *     the slot accounting is correct
+ */
+int test__bp_accounting(struct test *test __maybe_unused, int subtest __maybe_unused)
+{
+	int has_ioctl = detect_ioctl();
+	int wp_cnt = detect_cnt(false);
+	int bp_cnt = detect_cnt(true);
+	int share  = detect_share(wp_cnt, bp_cnt);
+
+	pr_debug("watchpoints count %d, breakpoints count %d, has_ioctl %d, share %d\n",
+		 wp_cnt, bp_cnt, has_ioctl, share);
+
+	if (!wp_cnt || !bp_cnt || !has_ioctl)
+		return TEST_SKIP;
+
+	return bp_accounting(wp_cnt, share);
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index fafa014240cd..38bf109ce106 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -115,6 +115,10 @@ static struct test generic_tests[] = {
 		.func = test__bp_signal_overflow,
 		.is_supported = test__bp_signal_is_supported,
 	},
+	{
+		.desc = "Breakpoint accounting",
+		.func = test__bp_accounting,
+	},
 	{
 		.desc = "Number of exit events of a simple workload",
 		.func = test__task_exit,
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 2862b80bc288..9f51edac44ae 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -58,6 +58,7 @@ int test__hists_link(struct test *test, int subtest);
 int test__python_use(struct test *test, int subtest);
 int test__bp_signal(struct test *test, int subtest);
 int test__bp_signal_overflow(struct test *test, int subtest);
+int test__bp_accounting(struct test *test, int subtest);
 int test__task_exit(struct test *test, int subtest);
 int test__mem(struct test *test, int subtest);
 int test__sw_clock_freq(struct test *test, int subtest);

[tip:perf/core] perf tests: Add breakpoint accounting/modify test

[tip-bot for Jiri Olsa]

Commit-ID:  032db28e5fa3594dfa3df585c54d8b612657f537
Gitweb:     https://git.kernel.org/tip/032db28e5fa3594dfa3df585c54d8b612657f537
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 12 Mar 2018 14:45:48 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Mar 2018 15:23:37 +0100

perf tests: Add breakpoint accounting/modify test

Adding test that:

  - detects the number of watch/break-points,
    skip test if any is missing
  - detects PERF_EVENT_IOC_MODIFY_ATTRIBUTES ioctl,
    skip test if it's missing
  - detects if watchpoints and breakpoints share
    same slots
  - create all possible watchpoints on cpu 0
  - change one of it to breakpoint
  - in case wp and bp do not share slots,
    we create another watchpoint to ensure
    the slot accounting is correct

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Milind Chabbi <chabbi.milind@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20180312134548.31532-9-jolsa@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/tests/Build          |   1 +
 tools/perf/tests/bp_account.c   | 195 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/builtin-test.c |   4 +
 tools/perf/tests/tests.h        |   1 +
 4 files changed, 201 insertions(+)

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 87bf3edb037c..62ca0174d5e1 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -20,6 +20,7 @@ perf-y += hists_cumulate.o
 perf-y += python-use.o
 perf-y += bp_signal.o
 perf-y += bp_signal_overflow.o
+perf-y += bp_account.o
 perf-y += task-exit.o
 perf-y += sw-clock.o
 perf-y += mmap-thread-lookup.o
diff --git a/tools/perf/tests/bp_account.c b/tools/perf/tests/bp_account.c
new file mode 100644
index 000000000000..2f75fa0c4fef
--- /dev/null
+++ b/tools/perf/tests/bp_account.c
@@ -0,0 +1,195 @@
+/*
+ * Powerpc needs __SANE_USERSPACE_TYPES__ before <linux/types.h> to select
+ * 'int-ll64.h' and avoid compile warnings when printing __u64 with %llu.
+ */
+#define __SANE_USERSPACE_TYPES__
+
+#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 <sys/ioctl.h>
+
+#include "tests.h"
+#include "debug.h"
+#include "perf.h"
+#include "cloexec.h"
+
+volatile long the_var;
+
+static noinline int test_function(void)
+{
+	return 0;
+}
+
+static int __event(bool is_x, void *addr, struct perf_event_attr *attr)
+{
+	int fd;
+
+	memset(attr, 0, sizeof(struct perf_event_attr));
+	attr->type = PERF_TYPE_BREAKPOINT;
+	attr->size = sizeof(struct perf_event_attr);
+
+	attr->config = 0;
+	attr->bp_type = is_x ? HW_BREAKPOINT_X : HW_BREAKPOINT_W;
+	attr->bp_addr = (unsigned long) addr;
+	attr->bp_len = sizeof(long);
+
+	attr->sample_period = 1;
+	attr->sample_type = PERF_SAMPLE_IP;
+
+	attr->exclude_kernel = 1;
+	attr->exclude_hv = 1;
+
+	fd = sys_perf_event_open(attr, -1, 0, -1,
+				 perf_event_open_cloexec_flag());
+	if (fd < 0) {
+		pr_debug("failed opening event %llx\n", attr->config);
+		return TEST_FAIL;
+	}
+
+	return fd;
+}
+
+static int wp_event(void *addr, struct perf_event_attr *attr)
+{
+	return __event(false, addr, attr);
+}
+
+static int bp_event(void *addr, struct perf_event_attr *attr)
+{
+	return __event(true, addr, attr);
+}
+
+static int bp_accounting(int wp_cnt, int share)
+{
+	struct perf_event_attr attr, attr_mod, attr_new;
+	int i, fd[wp_cnt], fd_wp, ret;
+
+	for (i = 0; i < wp_cnt; i++) {
+		fd[i] = wp_event((void *)&the_var, &attr);
+		TEST_ASSERT_VAL("failed to create wp\n", fd[i] != -1);
+		pr_debug("wp %d created\n", i);
+	}
+
+	attr_mod = attr;
+	attr_mod.bp_type = HW_BREAKPOINT_X;
+	attr_mod.bp_addr = (unsigned long) test_function;
+
+	ret = ioctl(fd[0], PERF_EVENT_IOC_MODIFY_ATTRIBUTES, &attr_mod);
+	TEST_ASSERT_VAL("failed to modify wp\n", ret == 0);
+
+	pr_debug("wp 0 modified to bp\n");
+
+	if (!share) {
+		fd_wp = wp_event((void *)&the_var, &attr_new);
+		TEST_ASSERT_VAL("failed to create max wp\n", fd_wp != -1);
+		pr_debug("wp max created\n");
+	}
+
+	for (i = 0; i < wp_cnt; i++)
+		close(fd[i]);
+
+	return 0;
+}
+
+static int detect_cnt(bool is_x)
+{
+	struct perf_event_attr attr;
+	void *addr = is_x ? test_function : (void *) &the_var;
+	int fd[100], cnt = 0, i;
+
+	while (1) {
+		fd[cnt] = __event(is_x, addr, &attr);
+
+		if (fd[cnt] < 0)
+			break;
+
+		if (cnt == 100) {
+			pr_debug("way too many debug registers, fix the test\n");
+			return 0;
+		}
+
+		cnt++;
+	}
+
+	for (i = 0; i < cnt; i++)
+		close(fd[i]);
+
+	return cnt;
+}
+
+static int detect_ioctl(void)
+{
+	struct perf_event_attr attr;
+	int fd, ret = 1;
+
+	fd = wp_event((void *) &the_var, &attr);
+	if (fd > 0) {
+		ret = ioctl(fd, PERF_EVENT_IOC_MODIFY_ATTRIBUTES, &attr);
+		close(fd);
+	}
+
+	return ret ? 0 : 1;
+}
+
+static int detect_share(int wp_cnt, int bp_cnt)
+{
+	struct perf_event_attr attr;
+	int i, fd[wp_cnt + bp_cnt], ret;
+
+	for (i = 0; i < wp_cnt; i++) {
+		fd[i] = wp_event((void *)&the_var, &attr);
+		TEST_ASSERT_VAL("failed to create wp\n", fd[i] != -1);
+	}
+
+	for (; i < (bp_cnt + wp_cnt); i++) {
+		fd[i] = bp_event((void *)test_function, &attr);
+		if (fd[i] == -1)
+			break;
+	}
+
+	ret = i != (bp_cnt + wp_cnt);
+
+	while (i--)
+		close(fd[i]);
+
+	return ret;
+}
+
+/*
+ * This test does following:
+ *   - detects the number of watch/break-points,
+ *     skip test if any is missing
+ *   - detects PERF_EVENT_IOC_MODIFY_ATTRIBUTES ioctl,
+ *     skip test if it's missing
+ *   - detects if watchpoints and breakpoints share
+ *     same slots
+ *   - create all possible watchpoints on cpu 0
+ *   - change one of it to breakpoint
+ *   - in case wp and bp do not share slots,
+ *     we create another watchpoint to ensure
+ *     the slot accounting is correct
+ */
+int test__bp_accounting(struct test *test __maybe_unused, int subtest __maybe_unused)
+{
+	int has_ioctl = detect_ioctl();
+	int wp_cnt = detect_cnt(false);
+	int bp_cnt = detect_cnt(true);
+	int share  = detect_share(wp_cnt, bp_cnt);
+
+	pr_debug("watchpoints count %d, breakpoints count %d, has_ioctl %d, share %d\n",
+		 wp_cnt, bp_cnt, has_ioctl, share);
+
+	if (!wp_cnt || !bp_cnt || !has_ioctl)
+		return TEST_SKIP;
+
+	return bp_accounting(wp_cnt, share);
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index fafa014240cd..38bf109ce106 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -115,6 +115,10 @@ static struct test generic_tests[] = {
 		.func = test__bp_signal_overflow,
 		.is_supported = test__bp_signal_is_supported,
 	},
+	{
+		.desc = "Breakpoint accounting",
+		.func = test__bp_accounting,
+	},
 	{
 		.desc = "Number of exit events of a simple workload",
 		.func = test__task_exit,
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 2862b80bc288..9f51edac44ae 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -58,6 +58,7 @@ int test__hists_link(struct test *test, int subtest);
 int test__python_use(struct test *test, int subtest);
 int test__bp_signal(struct test *test, int subtest);
 int test__bp_signal_overflow(struct test *test, int subtest);
+int test__bp_accounting(struct test *test, int subtest);
 int test__task_exit(struct test *test, int subtest);
 int test__mem(struct test *test, int subtest);
 int test__sw_clock_freq(struct test *test, int subtest);

Re: [PATCHv3 0/8] hw_breakpoint: Breakpoint modification fixes and new modify ioctl

[Ingo Molnar]

* Jiri Olsa <jolsa@kernel.org> wrote:

> Jiri Olsa (7):
>       hw_breakpoint: Pass bp_type directly as find_slot_idx argument
>       hw_breakpoint: Pass bp_type argument to __reserve_bp_slot|__release_bp_slot
>       hw_breakpoint: Add modify_bp_slot function
>       hw_breakpoint: Factor out __modify_user_hw_breakpoint function
>       hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint
>       perf/core: Move perf_event_attr::sample_max_stack into perf_copy_attr
>       perf tests: Add breakpoint accounting/modify test
> 
> Milind Chabbi (1):
>       perf/core: fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES.
> 
>  include/linux/hw_breakpoint.h         |   7 +++++
>  include/uapi/linux/perf_event.h       |  23 ++++++++-------
>  kernel/events/core.c                  |  54 ++++++++++++++++++++++++++++++++--
>  kernel/events/hw_breakpoint.c         | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++---------------------
>  tools/include/uapi/linux/perf_event.h |  23 ++++++++-------
>  tools/perf/tests/Build                |   1 +
>  tools/perf/tests/bp_account.c         | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/tests/builtin-test.c       |   4 +++
>  tools/perf/tests/tests.h              |   1 +
>  9 files changed, 365 insertions(+), 58 deletions(-)
>  create mode 100644 tools/perf/tests/bp_account.c

Note, there's a new PARISC build failure:

 home/mingo/tip/kernel/events/core.c:2858:2: error: implicit declaration of function 'modify_user_hw_breakpoint_check' [-Werror=implicit-function-declaration]
   err = modify_user_hw_breakpoint_check(bp, attr, true);

will only be able to push it out to -next once this is fixed.

Thanks,

	Ingo

Re: [PATCHv3 0/8] hw_breakpoint: Breakpoint modification fixes and new modify ioctl

[Jiri Olsa]

On Tue, Mar 13, 2018 at 07:37:47AM +0100, Ingo Molnar wrote:
> 
> * Jiri Olsa <jolsa@kernel.org> wrote:
> 
> > Jiri Olsa (7):
> >       hw_breakpoint: Pass bp_type directly as find_slot_idx argument
> >       hw_breakpoint: Pass bp_type argument to __reserve_bp_slot|__release_bp_slot
> >       hw_breakpoint: Add modify_bp_slot function
> >       hw_breakpoint: Factor out __modify_user_hw_breakpoint function
> >       hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint
> >       perf/core: Move perf_event_attr::sample_max_stack into perf_copy_attr
> >       perf tests: Add breakpoint accounting/modify test
> > 
> > Milind Chabbi (1):
> >       perf/core: fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES.
> > 
> >  include/linux/hw_breakpoint.h         |   7 +++++
> >  include/uapi/linux/perf_event.h       |  23 ++++++++-------
> >  kernel/events/core.c                  |  54 ++++++++++++++++++++++++++++++++--
> >  kernel/events/hw_breakpoint.c         | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++---------------------
> >  tools/include/uapi/linux/perf_event.h |  23 ++++++++-------
> >  tools/perf/tests/Build                |   1 +
> >  tools/perf/tests/bp_account.c         | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tools/perf/tests/builtin-test.c       |   4 +++
> >  tools/perf/tests/tests.h              |   1 +
> >  9 files changed, 365 insertions(+), 58 deletions(-)
> >  create mode 100644 tools/perf/tests/bp_account.c
> 
> Note, there's a new PARISC build failure:
> 
>  home/mingo/tip/kernel/events/core.c:2858:2: error: implicit declaration of function 'modify_user_hw_breakpoint_check' [-Werror=implicit-function-declaration]
>    err = modify_user_hw_breakpoint_check(bp, attr, true);
> 
> will only be able to push it out to -next once this is fixed.

ok, checking on that

jirka

Re: [PATCHv3 0/8] hw_breakpoint: Breakpoint modification fixes and new modify ioctl

[Jiri Olsa]

On Tue, Mar 13, 2018 at 10:28:01AM +0100, Jiri Olsa wrote:
> On Tue, Mar 13, 2018 at 07:37:47AM +0100, Ingo Molnar wrote:
> > 
> > * Jiri Olsa <jolsa@kernel.org> wrote:
> > 
> > > Jiri Olsa (7):
> > >       hw_breakpoint: Pass bp_type directly as find_slot_idx argument
> > >       hw_breakpoint: Pass bp_type argument to __reserve_bp_slot|__release_bp_slot
> > >       hw_breakpoint: Add modify_bp_slot function
> > >       hw_breakpoint: Factor out __modify_user_hw_breakpoint function
> > >       hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint
> > >       perf/core: Move perf_event_attr::sample_max_stack into perf_copy_attr
> > >       perf tests: Add breakpoint accounting/modify test
> > > 
> > > Milind Chabbi (1):
> > >       perf/core: fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES.
> > > 
> > >  include/linux/hw_breakpoint.h         |   7 +++++
> > >  include/uapi/linux/perf_event.h       |  23 ++++++++-------
> > >  kernel/events/core.c                  |  54 ++++++++++++++++++++++++++++++++--
> > >  kernel/events/hw_breakpoint.c         | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++---------------------
> > >  tools/include/uapi/linux/perf_event.h |  23 ++++++++-------
> > >  tools/perf/tests/Build                |   1 +
> > >  tools/perf/tests/bp_account.c         | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tools/perf/tests/builtin-test.c       |   4 +++
> > >  tools/perf/tests/tests.h              |   1 +
> > >  9 files changed, 365 insertions(+), 58 deletions(-)
> > >  create mode 100644 tools/perf/tests/bp_account.c
> > 
> > Note, there's a new PARISC build failure:
> > 
> >  home/mingo/tip/kernel/events/core.c:2858:2: error: implicit declaration of function 'modify_user_hw_breakpoint_check' [-Werror=implicit-function-declaration]
> >    err = modify_user_hw_breakpoint_check(bp, attr, true);
> > 
> > will only be able to push it out to -next once this is fixed.
> 
> ok, checking on that

should be this one.. I'm checking on s390 which is also
without breakpoint support, I'll send full patch after

jirka


---
diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index abeba094f080..6058c3844a76 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -101,8 +101,8 @@ static inline int
 modify_user_hw_breakpoint(struct perf_event *bp,
 			  struct perf_event_attr *attr)	{ return -ENOSYS; }
 static inline int
-__modify_user_hw_breakpoint(struct perf_event *bp,
-			    struct perf_event_attr *attr) { return -ENOSYS; }
+modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
+				bool check)	{ return -ENOSYS; }
 
 static inline struct perf_event *
 register_wide_hw_breakpoint_cpu(struct perf_event_attr *attr,

[PATCH] hw_breakpoint: Fix build for disabled CONFIG_HAVE_HW_BREAKPOINT

[Jiri Olsa]

On Tue, Mar 13, 2018 at 10:50:45AM +0100, Jiri Olsa wrote:
> On Tue, Mar 13, 2018 at 10:28:01AM +0100, Jiri Olsa wrote:
> > On Tue, Mar 13, 2018 at 07:37:47AM +0100, Ingo Molnar wrote:
> > > 
> > > * Jiri Olsa <jolsa@kernel.org> wrote:
> > > 
> > > > Jiri Olsa (7):
> > > >       hw_breakpoint: Pass bp_type directly as find_slot_idx argument
> > > >       hw_breakpoint: Pass bp_type argument to __reserve_bp_slot|__release_bp_slot
> > > >       hw_breakpoint: Add modify_bp_slot function
> > > >       hw_breakpoint: Factor out __modify_user_hw_breakpoint function
> > > >       hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint
> > > >       perf/core: Move perf_event_attr::sample_max_stack into perf_copy_attr
> > > >       perf tests: Add breakpoint accounting/modify test
> > > > 
> > > > Milind Chabbi (1):
> > > >       perf/core: fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES.
> > > > 
> > > >  include/linux/hw_breakpoint.h         |   7 +++++
> > > >  include/uapi/linux/perf_event.h       |  23 ++++++++-------
> > > >  kernel/events/core.c                  |  54 ++++++++++++++++++++++++++++++++--
> > > >  kernel/events/hw_breakpoint.c         | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++---------------------
> > > >  tools/include/uapi/linux/perf_event.h |  23 ++++++++-------
> > > >  tools/perf/tests/Build                |   1 +
> > > >  tools/perf/tests/bp_account.c         | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  tools/perf/tests/builtin-test.c       |   4 +++
> > > >  tools/perf/tests/tests.h              |   1 +
> > > >  9 files changed, 365 insertions(+), 58 deletions(-)
> > > >  create mode 100644 tools/perf/tests/bp_account.c
> > > 
> > > Note, there's a new PARISC build failure:
> > > 
> > >  home/mingo/tip/kernel/events/core.c:2858:2: error: implicit declaration of function 'modify_user_hw_breakpoint_check' [-Werror=implicit-function-declaration]
> > >    err = modify_user_hw_breakpoint_check(bp, attr, true);
> > > 
> > > will only be able to push it out to -next once this is fixed.
> > 
> > ok, checking on that
> 
> should be this one.. I'm checking on s390 which is also
> without breakpoint support, I'll send full patch after

and here it is

thanks,
jirka


---
We're missing stub for modify_user_hw_breakpoint_check function
when option CONFIG_HAVE_HW_BREAKPOINT is disabled. It was mixed
up in recent changes.

Fixes: f30b09b7f8ae ("perf/core: Implement fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/hw_breakpoint.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index abeba094f080..6058c3844a76 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -101,8 +101,8 @@ static inline int
 modify_user_hw_breakpoint(struct perf_event *bp,
 			  struct perf_event_attr *attr)	{ return -ENOSYS; }
 static inline int
-__modify_user_hw_breakpoint(struct perf_event *bp,
-			    struct perf_event_attr *attr) { return -ENOSYS; }
+modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
+				bool check)	{ return -ENOSYS; }
 
 static inline struct perf_event *
 register_wide_hw_breakpoint_cpu(struct perf_event_attr *attr,
-- 
2.13.6

Re: [PATCH] hw_breakpoint: Fix build for disabled CONFIG_HAVE_HW_BREAKPOINT

[Ingo Molnar]

* Jiri Olsa <jolsa@redhat.com> wrote:

> > should be this one.. I'm checking on s390 which is also
> > without breakpoint support, I'll send full patch after
> 
> and here it is

Yeah, note that I back-merged this into the series so there's no separate commit 
for the fix.

Thanks,

	Ingo