perf, persistent: Add persistent events

This patch set implements the necessary kernel changes for persistent events.

Persistent events run standalone in the system without the need of a controlling process that holds an event's file descriptor. The events are always enabled and collect data samples in a ring buffer. Processes may connect to existing persistent events using the perf_event_open() syscall. For this the syscall must be configured using the new PERF_TYPE_PERSISTENT event type and a unique event identifier specified in attr.config. The id is propagated in sysfs or using ioctl (see below).

Persistent event buffers may be accessed with mmap() in the same way as for any other event. Since the buffers may be used by multiple processes at the same time, there is only read-only access to them. Currently there is only support for per-cpu events, thus root access is needed too.

Persistent events are visible in sysfs. They are added or removed dynamically. With the information in sysfs userland knows about how to setup the perf_event attribute of a persistent event. Since a persistent event always has the persistent flag set, a way is needed to express this in sysfs. A new syntax is used for this. With 'attr<num>:<mask>' any bit in the attribute structure may be set in a similar way as using 'config<num>', but <num> is an index that points to the u64 value to change within the attribute.

For persistent events the persistent flag (bit 23 of flag field in struct perf_event_attr) needs to be set which is expressed in sysfs with "attr5:23". E.g. the mce_record event is described in sysfs as follows:

/sys/bus/event_source/devices/persistent/events/mce_record:persistent,config=106 /sys/bus/event_source/devices/persistent/format/persistent:attr5:23

Note that perf tools need to support the 'attr<num>' syntax that is added in a separate patch set. With it we are able to run perf tool commands to read persistent events, e.g.:

 # perf record -e persistent/mce_record/ sleep 10
 # perf top -e persistent/mce_record/

In general the new syntax is flexible to describe with sysfs any event to be setup by perf tools.

There are ioctl functions to control persistent events that can be used to detach or attach an event to or from a process. The PERF_EVENT_IOC_DETACH ioctl call makes an event persistent. The perf_event_open() syscall can be used to re-open the event by any process. The PERF_EVENT_IOC_ATTACH ioctl attaches the event again so that it is removed after closing the event's fd.

The patches base on the originally work from Borislav Petkov.

This version 3 of the patch set is a complete rework of the code. There are the following major changes:

* new event type PERF_TYPE_PERSISTENT introduced,

* support for all type of events,

* unique event ids,

* improvements in reference counting and locking,

* ioctl functions are added to control persistency,

* the sysfs implementation now uses variable list size.

This should address most issues discussed during last review of version 2. The following is unresolved yet and can be added later on top of this patches, if necessary:

* support for per-task events (also allowing non-root access),

* creation of persistent events for disabled cpus,

* make event persistent with already open (mmap'ed) buffers,

* make event persistent while creating it.

First patches contain some rework of the perf mmap code to reuse it for persistent events.

Also note that patch 12 (ioctl functions to control persistency) is RFC and untested. A perf tools implementation for this is missing and some ideas are needed how this could be integrated, esp. in something like perf trace or so.

All patches can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git persistent-v3

Note: I will resent the perf tools patch necessary to use persistent events.

-Robert

Borislav Petkov (1):
mce, x86: Enable persistent events

Robert Richter (11):
perf, mmap: Factor out ring_buffer_detach_all() perf, mmap: Factor out try_get_event()/put_event() perf, mmap: Factor out perf_alloc/free_rb()
perf, mmap: Factor out perf_get_fd()
perf: Add persistent events
perf, persistent: Implementing a persistent pmu perf, persistent: Exposing persistent events using sysfs perf, persistent: Use unique event ids
perf, persistent: Implement reference counter for events perf, persistent: Dynamically resize list of sysfs entries [RFC] perf, persistent: ioctl functions to control persistency

 .../testing/sysfs-bus-event_source-devices-format  |  43 +-
 arch/x86/kernel/cpu/mcheck/mce.c                   |  19 +
 include/linux/perf_event.h                         |  12 +-
 include/uapi/linux/perf_event.h                    |   6 +-
 kernel/events/Makefile                             |   2 +-
 kernel/events/core.c                               | 210 +++++---
 kernel/events/internal.h                           |  20 +
 kernel/events/persistent.c                         | 563 +++++++++++++++++++++
 8 files changed, 779 insertions(+), 96 deletions(-)
 create mode 100644 kernel/events/persistent.c

1.8.3.2

From: Robert Richter <[email protected]>

Factor out a function to detach all events from a ringbuffer. No functional changes.

---
 kernel/events/core.c | 82 ++++++++++++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 38 deletions(-)

index 928fae7c..5dcc5fe 100644
@@ -3775,6 +3775,49 @@ static void ring_buffer_detach(struct perf_event *event, struct ring_buffer *rb)
 	spin_unlock_irqrestore(&rb->event_lock, flags);
 }
 
+static void ring_buffer_detach_all(struct ring_buffer *rb)
+{
+	struct perf_event *event;
+again:
+	rcu_read_lock();
+	list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
+		if (!atomic_long_inc_not_zero(&event->refcount)) {
+			/*
+			 * This event is en-route to free_event() which will
+			 * detach it and remove it from the list.
+			 */
+			continue;
+		}
+		rcu_read_unlock();
+
+		mutex_lock(&event->mmap_mutex);
+		/*
+		 * Check we didn't race with perf_event_set_output() which can
+		 * swizzle the rb from under us while we were waiting to
+		 * acquire mmap_mutex.
+		 *
+		 * If we find a different rb; ignore this event, a next
+		 * iteration will no longer find it on the list. We have to
+		 * still restart the iteration to make sure we're not now
+		 * iterating the wrong list.
+		 */
+		if (event->rb == rb) {
+			rcu_assign_pointer(event->rb, NULL);
+			ring_buffer_detach(event, rb);
+			ring_buffer_put(rb); /* can't be last, we still have one */
+		}
+		mutex_unlock(&event->mmap_mutex);
+		put_event(event);
+
+		/*
+		 * Restart the iteration; either we're on the wrong list or
+		 * destroyed its integrity by doing a deletion.
+		 */
+		goto again;
+	}
+	rcu_read_unlock();
+}
+
 static void ring_buffer_wakeup(struct perf_event *event)
 {
 	struct ring_buffer *rb;

@@ -3867,44 +3910,7 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 	 * into the now unreachable buffer. Somewhat complicated by the
 	 * fact that rb::event_lock otherwise nests inside mmap_mutex.
 	 */
-again:
-	rcu_read_lock();
-	list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
-		if (!atomic_long_inc_not_zero(&event->refcount)) {
-			/*
-			 * This event is en-route to free_event() which will
-			 * detach it and remove it from the list.
-			 */
-			continue;
-		}
-		rcu_read_unlock();
-
-		mutex_lock(&event->mmap_mutex);
-		/*
-		 * Check we didn't race with perf_event_set_output() which can
-		 * swizzle the rb from under us while we were waiting to
-		 * acquire mmap_mutex.
-		 *
-		 * If we find a different rb; ignore this event, a next
-		 * iteration will no longer find it on the list. We have to
-		 * still restart the iteration to make sure we're not now
-		 * iterating the wrong list.
-		 */
-		if (event->rb == rb) {
-			rcu_assign_pointer(event->rb, NULL);
-			ring_buffer_detach(event, rb);
-			ring_buffer_put(rb); /* can't be last, we still have one */
-		}
-		mutex_unlock(&event->mmap_mutex);
-		put_event(event);
-
-		/*
-		 * Restart the iteration; either we're on the wrong list or
-		 * destroyed its integrity by doing a deletion.
-		 */
-		goto again;
-	}
-	rcu_read_unlock();
+	ring_buffer_detach_all(rb);
 
 	/*
 	 * It could be there's still a few 0-ref events on the list; they'll

1.8.3.2

From: Robert Richter <[email protected]>

Implement try_get_event() as counter part to put_event(). Put both in internal.h to make it available to other perf files.

---
 kernel/events/core.c     |  9 +++------
 kernel/events/internal.h | 12 ++++++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

index 5dcc5fe..c9a5d4c 100644
@@ -3242,13 +3242,10 @@ EXPORT_SYMBOL_GPL(perf_event_release_kernel);
 /*
  * Called when the last reference to the file is gone.
  */
-static void put_event(struct perf_event *event)
+void __put_event(struct perf_event *event)
 {
 	struct task_struct *owner;
 
-	if (!atomic_long_dec_and_test(&event->refcount))
-		return;
-
 	rcu_read_lock();
 	owner = ACCESS_ONCE(event->owner);
 	/*

@@ -3781,7 +3778,7 @@ static void ring_buffer_detach_all(struct ring_buffer *rb)
 again:
 	rcu_read_lock();
 	list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
-		if (!atomic_long_inc_not_zero(&event->refcount)) {
+		if (!try_get_event(event)) {
 			/*
 			 * This event is en-route to free_event() which will
 			 * detach it and remove it from the list.

@@ -7445,7 +7442,7 @@ inherit_event(struct perf_event *parent_event,
 	if (IS_ERR(child_event))
 		return child_event;
 
-	if (!atomic_long_inc_not_zero(&parent_event->refcount)) {
+	if (!try_get_event(parent_event)) {
 		free_event(child_event);
 		return NULL;
 	}

index ca65997..96a07d2 100644
@@ -178,4 +178,16 @@ static inline bool arch_perf_have_user_stack_dump(void)
 #define perf_user_stack_pointer(regs) 0
 #endif /* CONFIG_HAVE_PERF_USER_STACK_DUMP */
 
+static inline bool try_get_event(struct perf_event *event)
+{
+	return atomic_long_inc_not_zero(&event->refcount) != 0;
+}
+extern void __put_event(struct perf_event *event);
+static inline void put_event(struct perf_event *event)
+{
+	if (!atomic_long_dec_and_test(&event->refcount))
+		return;
+	__put_event(event);
+}
+
 #endif /* _KERNEL_EVENTS_INTERNAL_H */

1.8.3.2

From: Robert Richter <[email protected]>

Factor out code to allocate and deallocate ringbuffers. We need this later to setup the sampling buffer for persistent events.

While at this, replacing get_current_user() with get_uid(user).

---
 kernel/events/core.c     | 75 +++++++++++++++++++++++++++++-------------------
 kernel/events/internal.h |  3 ++
 2 files changed, 48 insertions(+), 30 deletions(-)

index c9a5d4c..24810d5 100644
@@ -3124,8 +3124,44 @@ static void free_event_rcu(struct rcu_head *head)
 }
 
 static void ring_buffer_put(struct ring_buffer *rb);
+static void ring_buffer_attach(struct perf_event *event, struct ring_buffer *rb);
 static void ring_buffer_detach(struct perf_event *event, struct ring_buffer *rb);
 
+/*
+ * Must be called with &event->mmap_mutex held. event->rb must be
+ * NULL. perf_alloc_rb() requires &event->mmap_count to be incremented
+ * on success which corresponds to &rb->mmap_count that is initialized
+ * with 1.
+ */
+int perf_alloc_rb(struct perf_event *event, int nr_pages, int flags)
+{
+	struct ring_buffer *rb;
+
+	rb = rb_alloc(nr_pages,
+		event->attr.watermark ? event->attr.wakeup_watermark : 0,
+		event->cpu, flags);
+	if (!rb)
+		return -ENOMEM;
+
+	atomic_set(&rb->mmap_count, 1);
+	ring_buffer_attach(event, rb);
+	rcu_assign_pointer(event->rb, rb);
+
+	perf_event_update_userpage(event);
+
+	return 0;
+}
+
+/* Must be called with &event->mmap_mutex held. event->rb must be set. */
+void perf_free_rb(struct perf_event *event)
+{
+	struct ring_buffer *rb = event->rb;
+
+	rcu_assign_pointer(event->rb, NULL);
+	ring_buffer_detach(event, rb);
+	ring_buffer_put(rb);
+}
+
 static void unaccount_event_cpu(struct perf_event *event, int cpu)
 {
 	if (event->parent)

@@ -3177,6 +3213,7 @@ static void __free_event(struct perf_event *event)
 
 	call_rcu(&event->rcu_head, free_event_rcu);
 }
+
 static void free_event(struct perf_event *event)
 {
 	irq_work_sync(&event->pending);

@@ -3184,8 +3221,6 @@ static void free_event(struct perf_event *event)
 	unaccount_event(event);
 
 	if (event->rb) {
-		struct ring_buffer *rb;
-
 		/*
 		 * Can happen when we close an event with re-directed output.
 		 *

@@ -3193,12 +3228,8 @@ static void free_event(struct perf_event *event)
 		 * over us; possibly making our ring_buffer_put() the last.
 		 */
 		mutex_lock(&event->mmap_mutex);
-		rb = event->rb;
-		if (rb) {
-			rcu_assign_pointer(event->rb, NULL);
-			ring_buffer_detach(event, rb);
-			ring_buffer_put(rb); /* could be last */
-		}
+		if (event->rb)
+			perf_free_rb(event);
 		mutex_unlock(&event->mmap_mutex);
 	}
 

@@ -3798,11 +3829,8 @@ static void ring_buffer_detach_all(struct ring_buffer *rb)
 		 * still restart the iteration to make sure we're not now
 		 * iterating the wrong list.
 		 */
-		if (event->rb == rb) {
-			rcu_assign_pointer(event->rb, NULL);
-			ring_buffer_detach(event, rb);
-			ring_buffer_put(rb); /* can't be last, we still have one */
-		}
+		if (event->rb == rb)
+			perf_free_rb(event);
 		mutex_unlock(&event->mmap_mutex);
 		put_event(event);
 

@@ -3938,7 +3966,6 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 	unsigned long user_locked, user_lock_limit;
 	struct user_struct *user = current_user();
 	unsigned long locked, lock_limit;
-	struct ring_buffer *rb;
 	unsigned long vma_size;
 	unsigned long nr_pages;
 	long user_extra, extra;

@@ -4022,27 +4049,15 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 	if (vma->vm_flags & VM_WRITE)
 		flags |= RING_BUFFER_WRITABLE;
 
-	rb = rb_alloc(nr_pages, 
-		event->attr.watermark ? event->attr.wakeup_watermark : 0,
-		event->cpu, flags);
-
-	if (!rb) {
-		ret = -ENOMEM;
+	ret = perf_alloc_rb(event, nr_pages, flags);
+	if (ret)
 		goto unlock;
-	}
 
-	atomic_set(&rb->mmap_count, 1);
-	rb->mmap_locked = extra;
-	rb->mmap_user = get_current_user();
+	event->rb->mmap_locked = extra;
+	event->rb->mmap_user = get_uid(user);
 
 	atomic_long_add(user_extra, &user->locked_vm);
 	vma->vm_mm->pinned_vm += extra;
-
-	ring_buffer_attach(event, rb);
-	rcu_assign_pointer(event->rb, rb);
-
-	perf_event_update_userpage(event);
-
 unlock:
 	if (!ret)
 		atomic_inc(&event->mmap_count);

index 96a07d2..8ddaf57 100644
@@ -190,4 +190,7 @@ static inline void put_event(struct perf_event *event)
 	__put_event(event);
 }
 
+extern int perf_alloc_rb(struct perf_event *event, int nr_pages, int flags);
+extern void perf_free_rb(struct perf_event *event);
+
 #endif /* _KERNEL_EVENTS_INTERNAL_H */

1.8.3.2

From: Robert Richter <[email protected]>

This new function creates a new fd for an event. We need this later to get a fd from a persistent event.

---
 kernel/events/core.c     | 13 ++++++++-----
 kernel/events/internal.h |  1 +
 2 files changed, 9 insertions(+), 5 deletions(-)

index 24810d5..932acc6 100644
@@ -4100,6 +4100,11 @@ static const struct file_operations perf_fops = {
 	.fasync			= perf_fasync,
 };
 
+int perf_get_fd(struct perf_event *event)
+{
+	return anon_inode_getfd("[perf_event]", &perf_fops, event, O_RDWR);
+}
+
 /*
  * Perf event wakeup
  *

@@ -6868,7 +6873,6 @@ SYSCALL_DEFINE5(perf_event_open,
 	struct perf_event *event, *sibling;
 	struct perf_event_attr attr;
 	struct perf_event_context *ctx;
-	struct file *event_file = NULL;
 	struct fd group = {NULL, 0};
 	struct task_struct *task = NULL;
 	struct pmu *pmu;

@@ -7025,9 +7029,9 @@ SYSCALL_DEFINE5(perf_event_open,
 			goto err_context;
 	}
 
-	event_file = anon_inode_getfile("[perf_event]", &perf_fops, event, O_RDWR);
-	if (IS_ERR(event_file)) {
-		err = PTR_ERR(event_file);
+	event_fd = perf_get_fd(event);
+	if (event_fd < 0) {
+		err = event_fd;
 		goto err_context;
 	}
 

@@ -7093,7 +7097,6 @@ SYSCALL_DEFINE5(perf_event_open,
 	 * perf_group_detach().
 	 */
 	fdput(group);
-	fd_install(event_fd, event_file);
 	return event_fd;
 
 err_context:

index 8ddaf57..d8708aa 100644
@@ -192,5 +192,6 @@ static inline void put_event(struct perf_event *event)
 
 extern int perf_alloc_rb(struct perf_event *event, int nr_pages, int flags);
 extern void perf_free_rb(struct perf_event *event);
+extern int perf_get_fd(struct perf_event *event);
 
 #endif /* _KERNEL_EVENTS_INTERNAL_H */

1.8.3.2

From: Robert Richter <[email protected]>

Add the needed pieces for persistent events which makes them process-agnostic. Also, make their buffers read-only when mmaping them from userspace.

Add a barebones implementation for registering persistent events with perf. For that, we don't destroy the buffers when they're unmapped; also, we map them read-only so that multiple agents can access them.

Also, we allocate the event buffers at event init time and not at mmap time so that we can log samples into them regardless of whether there are readers in userspace or not.

Multiple events from different cpus may map to a single persistent event entry which has a unique identifier. The identifier allows to access the persistent event with the perf_event_open() syscall. For this the new event type PERF_TYPE_PERSISTENT must be set with its id specified in attr.config. Currently there is only support for per-cpu events. Also, root access is required.

Since the buffers are shared, the set_output ioctl may not be used in conjunction with persistent events.

This patch only supports trace_points, support for all event types is implemented in a later patch.

Based on patch set from Borislav Petkov <[email protected]>.

---
 include/linux/perf_event.h      |  12 ++-
 include/uapi/linux/perf_event.h |   4 +-
 kernel/events/Makefile          |   2 +-
 kernel/events/core.c            |  37 +++++--
 kernel/events/internal.h        |   2 +
 kernel/events/persistent.c      | 221 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 266 insertions(+), 12 deletions(-)
 create mode 100644 kernel/events/persistent.c

index c43f6ea..1a62a25 100644
@@ -436,6 +436,8 @@ struct perf_event {
 	struct perf_cgroup		*cgrp; /* cgroup event is attach to */
 	int				cgrp_defer_enabled;
 #endif
+	struct list_head		pevent_entry;	/* persistent event */
+	int				pevent_id;
 
 #endif /* CONFIG_PERF_EVENTS */
 };

@@ -765,7 +767,7 @@ extern void perf_event_enable(struct perf_event *event);
 extern void perf_event_disable(struct perf_event *event);
 extern int __perf_event_disable(void *info);
 extern void perf_event_task_tick(void);
-#else
+#else /* !CONFIG_PERF_EVENTS */
 static inline void
 perf_event_task_sched_in(struct task_struct *prev,
 			 struct task_struct *task)			{ }

@@ -805,7 +807,7 @@ static inline void perf_event_enable(struct perf_event *event)		{ }
 static inline void perf_event_disable(struct perf_event *event)		{ }
 static inline int __perf_event_disable(void *info)			{ return -1; }
 static inline void perf_event_task_tick(void)				{ }
-#endif
+#endif /* !CONFIG_PERF_EVENTS */
 
 #if defined(CONFIG_PERF_EVENTS) && defined(CONFIG_NO_HZ_FULL)
 extern bool perf_event_can_stop_tick(void);

@@ -819,6 +821,12 @@ extern void perf_restore_debug_store(void);
 static inline void perf_restore_debug_store(void)			{ }
 #endif
 
+#if defined(CONFIG_PERF_EVENTS) && defined(CONFIG_EVENT_TRACING)
+extern int perf_add_persistent_tp(struct ftrace_event_call *tp);
+#else
+static inline int perf_add_persistent_tp(void *tp)			{ return -ENOENT; }
+#endif
+
 #define perf_output_put(handle, x) perf_output_copy((handle), &(x), sizeof(x))
 
 /*

index 62c25a2..2b84b97 100644
@@ -32,6 +32,7 @@ enum perf_type_id {
 	PERF_TYPE_HW_CACHE			= 3,
 	PERF_TYPE_RAW				= 4,
 	PERF_TYPE_BREAKPOINT			= 5,
+	PERF_TYPE_PERSISTENT			= 6,
 
 	PERF_TYPE_MAX,				/* non-ABI */
 };

@@ -275,8 +276,9 @@ struct perf_event_attr {
 
 				exclude_callchain_kernel : 1, /* exclude kernel callchains */
 				exclude_callchain_user   : 1, /* exclude user callchains */
+				persistent     :  1, /* always-on event */
 
-				__reserved_1   : 41;
+				__reserved_1   : 40;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */

index 103f5d1..70990d5 100644
@@ -2,7 +2,7 @@ ifdef CONFIG_FUNCTION_TRACER
 CFLAGS_REMOVE_core.o = -pg
 endif
 
-obj-y := core.o ring_buffer.o callchain.o
+obj-y := core.o ring_buffer.o callchain.o persistent.o
 
 obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
 obj-$(CONFIG_UPROBES) += uprobes.o

index 932acc6..d9d6e67 100644
@@ -3982,6 +3982,9 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 	if (!(vma->vm_flags & VM_SHARED))
 		return -EINVAL;
 
+	if (event->attr.persistent && (vma->vm_flags & VM_WRITE))
+		return -EACCES;
+
 	vma_size = vma->vm_end - vma->vm_start;
 	nr_pages = (vma_size / PAGE_SIZE) - 1;
 

@@ -4007,6 +4010,11 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 			goto unlock;
 		}
 
+		if (!event->rb->overwrite && vma->vm_flags & VM_WRITE) {
+			ret = -EACCES;
+			goto unlock;
+		}
+
 		if (!atomic_inc_not_zero(&event->rb->mmap_count)) {
 			/*
 			 * Raced against perf_mmap_close() through

@@ -5845,7 +5853,7 @@ static struct pmu perf_tracepoint = {
 	.event_idx	= perf_swevent_event_idx,
 };
 
-static inline void perf_tp_register(void)
+static inline void perf_register_tp(void)
 {
 	perf_pmu_register(&perf_tracepoint, "tracepoint", PERF_TYPE_TRACEPOINT);
 }

@@ -5875,18 +5883,14 @@ static void perf_event_free_filter(struct perf_event *event)
 
 #else
 
-static inline void perf_tp_register(void)
-{
-}
+static inline void perf_register_tp(void) { }
 
 static int perf_event_set_filter(struct perf_event *event, void __user *arg)
 {
 	return -ENOENT;
 }
 
-static void perf_event_free_filter(struct perf_event *event)
-{
-}
+static void perf_event_free_filter(struct perf_event *event) { }
 
 #endif /* CONFIG_EVENT_TRACING */
 

@@ -6574,6 +6578,7 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
 	INIT_LIST_HEAD(&event->event_entry);
 	INIT_LIST_HEAD(&event->sibling_list);
 	INIT_LIST_HEAD(&event->rb_entry);
+	INIT_LIST_HEAD(&event->pevent_entry);
 
 	init_waitqueue_head(&event->waitq);
 	init_irq_work(&event->pending, perf_pending_event);

@@ -6831,6 +6836,13 @@ perf_event_set_output(struct perf_event *event, struct perf_event *output_event)
 			goto unlock;
 	}
 
+	/* Don't redirect read-only (persistent) events. */
+	ret = -EACCES;
+	if (old_rb && !old_rb->overwrite)
+		goto unlock;
+	if (rb && !rb->overwrite)
+		goto unlock;
+
 	if (old_rb)
 		ring_buffer_detach(event, old_rb);
 

@@ -6888,6 +6900,14 @@ SYSCALL_DEFINE5(perf_event_open,
 	if (err)
 		return err;
 
+	/* return fd for an existing persistent event */
+	if (attr.type == PERF_TYPE_PERSISTENT)
+		return perf_get_persistent_event_fd(cpu, attr.config);
+
+	/* put event into persistent state (not yet supported) */
+	if (attr.persistent)
+		return -EOPNOTSUPP;
+
 	if (!attr.exclude_kernel) {
 		if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
 			return -EACCES;

@@ -7828,7 +7848,8 @@ void __init perf_event_init(void)
 	perf_pmu_register(&perf_swevent, "software", PERF_TYPE_SOFTWARE);
 	perf_pmu_register(&perf_cpu_clock, NULL, -1);
 	perf_pmu_register(&perf_task_clock, NULL, -1);
-	perf_tp_register();
+	perf_register_tp();
+	perf_register_persistent();
 	perf_cpu_notifier(perf_cpu_notify);
 	register_reboot_notifier(&perf_reboot_notifier);
 

index d8708aa..94c3f73 100644
@@ -193,5 +193,7 @@ static inline void put_event(struct perf_event *event)
 extern int perf_alloc_rb(struct perf_event *event, int nr_pages, int flags);
 extern void perf_free_rb(struct perf_event *event);
 extern int perf_get_fd(struct perf_event *event);
+extern int perf_get_persistent_event_fd(int cpu, int id);
+extern void __init perf_register_persistent(void);
 
 #endif /* _KERNEL_EVENTS_INTERNAL_H */

1.8.3.2

From: Borislav Petkov <[email protected]>

... for MCEs collection.

---
 arch/x86/kernel/cpu/mcheck/mce.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

index 87a65c9..ffa227b 100644
@@ -1990,6 +1990,25 @@ int __init mcheck_init(void)
 	return 0;
 }
 
+#ifdef CONFIG_EVENT_TRACING
+
+int __init mcheck_init_tp(void)
+{
+	int ret = perf_add_persistent_tp(&event_mce_record);
+
+	if (ret && ret != -ENOENT)
+		pr_err("Error adding MCE persistent event: %d\n", ret);
+
+	return ret;
+}
+/*
+ * We can't run earlier because persistent events uses anon_inode_getfile and
+ * its anon_inode_mnt gets initialized as a fs_initcall.
+ */
+fs_initcall_sync(mcheck_init_tp);
+
+#endif /* CONFIG_EVENT_TRACING */
+
 /*
  * mce_syscore: PM support
  */

1.8.3.2

From: Robert Richter <[email protected]>

We want to use the kernel's pmu design to later expose persistent events via sysfs to userland. Initially implement a persistent pmu.

The format syntax is introduced allowing to set bits anywhere in struct perf_event_attr. This is used in this case to set the persistent flag (attr5:23). The syntax is attr<num> where num is the index of the u64 array in struct perf_event_attr. Otherwise syntax is same as for config<num>.

Patches that implement this functionality for perf tools are sent in a separate patchset.

---
 kernel/events/persistent.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

index 926654f..ede95ab 100644
@@ -12,6 +12,7 @@ struct pevent {
 	int		id;
 };
 
+static struct pmu persistent_pmu;
 static DEFINE_PER_CPU(struct list_head, pevents);
 static DEFINE_PER_CPU(struct mutex, pevents_lock);
 

@@ -210,10 +211,43 @@ int perf_get_persistent_event_fd(int cpu, int id)
 	return event_fd;
 }
 
+PMU_FORMAT_ATTR(persistent, "attr5:23");
+
+static struct attribute *persistent_format_attrs[] = {
+	&format_attr_persistent.attr,
+	NULL,
+};
+
+static struct attribute_group persistent_format_group = {
+	.name = "format",
+	.attrs = persistent_format_attrs,
+};
+
+static const struct attribute_group *persistent_attr_groups[] = {
+	&persistent_format_group,
+	NULL,
+};
+
+static int persistent_pmu_init(struct perf_event *event)
+{
+	if (persistent_pmu.type != event->attr.type)
+		return -ENOENT;
+
+	/* Not a persistent event. */
+	return -EFAULT;
+}
+
+static struct pmu persistent_pmu = {
+	.event_init	= persistent_pmu_init,
+	.attr_groups	= persistent_attr_groups,
+};
+
 void __init perf_register_persistent(void)
 {
 	int cpu;
 
+	perf_pmu_register(&persistent_pmu, "persistent", PERF_TYPE_PERSISTENT);
+
 	for_each_possible_cpu(cpu) {
 		INIT_LIST_HEAD(&per_cpu(pevents, cpu));
 		mutex_init(&per_cpu(pevents_lock, cpu));

1.8.3.2

From: Robert Richter <[email protected]>

Expose persistent events in the system to userland using sysfs. Perf tools are able to read existing pmu events from sysfs. Now we use a persistent pmu as an event container containing all registered persistent events of the system. This patch adds dynamically registration of persistent events to sysfs. E.g. something like this:

/sys/bus/event_source/devices/persistent/events/mce_record:persistent,config=106 /sys/bus/event_source/devices/persistent/format/persistent:attr5:23

Perf tools need to support the attr<num> syntax that is added in a separate patch set. With it we are able to run perf tool commands to read persistent events, e.g.:

 # perf record -e persistent/mce_record/ sleep 10
 # perf top -e persistent/mce_record/

---
 .../testing/sysfs-bus-event_source-devices-format  | 43 ++++++++++++----
 kernel/events/persistent.c                         | 60 ++++++++++++++++++++++
 2 files changed, 92 insertions(+), 11 deletions(-)

index 77f47ff..2dbb911 100644
@@ -1,13 +1,14 @@
-Where:		/sys/bus/event_source/devices/<dev>/format
+Where:		/sys/bus/event_source/devices/<pmu>/format/<name>
 Date:		January 2012
-Kernel Version: 3.3
+Kernel Version:	3.3
+		3.xx	(added attr<index>:<bits>)
 Contact:	Jiri Olsa <[email protected]>
-Description:
-		Attribute group to describe the magic bits that go into
-		perf_event_attr::config[012] for a particular pmu.
-		Each attribute of this group defines the 'hardware' bitmask
-		we want to export, so that userspace can deal with sane
-		name/value pairs.
+
+Description:	Define formats for bit ranges in perf_event_attr
+
+		Attribute group to describe the magic bits that go
+		into struct perf_event_attr for a particular pmu. Bit
+		range may be any bit mask of an u64 (bits 0 to 63).
 
 		Userspace must be prepared for the possibility that attributes
 		define overlapping bit ranges. For example:

@@ -15,6 +16,26 @@ Contact:	Jiri Olsa <[email protected]>
 			attr2 = 'config:0-7'
 			attr3 = 'config:12-35'
 
-		Example: 'config1:1,6-10,44'
-		Defines contents of attribute that occupies bits 1,6-10,44 of
-		perf_event_attr::config1.
+		Syntax			Description
+
+		config[012]*:<bits>	Each attribute of this group
+					defines the 'hardware' bitmask
+					we want to export, so that
+					userspace can deal with sane
+					name/value pairs.
+
+		attr<index>:<bits>	Set any field of the event
+					attribute. The index is a
+					decimal number that specifies
+					the u64 value to be set within
+					struct perf_event_attr.
+
+		Examples:
+
+		'config1:1,6-10,44'	Defines contents of attribute
+					that occupies bits 1,6-10,44
+					of perf_event_attr::config1.
+
+		'attr5:23'		Define the persistent event
+					flag (bit 23 of the attribute
+					flags)

1.8.3.2

...

    attr_precise_ip:0-1

It's not intended to be used by humans. ;) It is more for format definitions that are provided by the kernel and that are parsed by (perf) tools. Of course, a human could dig into it to figure out that's going on.

I thought it would be clear enough to refer to struct perf_event_attr. Since the index usually starts with 0 as in the config fields, I assumed this was clear in this case too. Though this can be documented better.

It is the endianness used in the syscall. Handled in the same way as for the config fields. I don't see where this could be an issue.

The format directories in /sys/bus/event_source/devices/*/format/ are already there to make it human readable. A user never has to deal directly with syntax provided there and may use already the abstractions for the event syntax.

The problem with this is that you have to implement this in the event parser of perf tools. Thus, you need to update the parser for any other syntax you want to use. This is not necessary with my implementation. It already provides the above. The pmu driver just need to add the sysfs entry.

.../format/precise_ip = attr5:15-16

Then, -e cycles,precise_ip=1 is the same as -e cycles:p. Looks very human readable?

All this without updating perf tools.

Simply test this as follows:

 # cp -rp /sys/devices/cpu/format/ /dev/shm/
 # echo attr5:15-16 > /dev/shm/format/precise_ip
 # mount --bind /dev/shm/format/ /sys/devices/cpu/format/
 # find /sys/devices/cpu/format/
 /sys/devices/cpu/format/
 /sys/devices/cpu/format/precise_ip
 /sys/devices/cpu/format/umask
 /sys/devices/cpu/format/event
 /sys/devices/cpu/format/cmask
 /sys/devices/cpu/format/edge
 /sys/devices/cpu/format/inv
 # perf record -e cycles,precise_ip=1 sleep 1

Works out-of-the-box...

It's the whole intention of the new syntax that the event parser never needs to be modified again for new attribute flags or any other settings of the attributes. Now the syntax is also capable to describe any event setup. Also consider that different architectures may provide different syntax. In this case there is no need for arch-specific code in the tools implementation, all is just brought by sysfs.

That's a different story. Guess there is no way back anymore now. Though we are in a state all this is handable and covered by perf tools.

-Robert

Make no assumptions when documenting. When I as a user have to dig around the kernel source tree to find out what is going on then the documentation is lacking.

C bitfields go opposite ways on big-endian vs little-endian systems. This has come up with some of the bitfields in the sample buffers. It doesn't matter if you just use the bitfields, but if you're trying to poke single bits into opaque 64-bit blobs it might be an issue.

The format directory for now was only for the "config" fields which traditionally were needed to specify an event, that is all.

Things get a lot more complex if arbitrary subsets of the the perf_attr structure start getting exported.

I see. Are you going to update the parsers for programs that also try to read these values, like trinity?

Or is the perf tool special because it is in the kernel?

So are we going to admit the ABI is "it doesn't break perf" after all?

the *perf* event parser never needs to be modified again maybe.

In any case, Andi Kleen also has some patches to this effect so you might want to co-ordinate your efforts. In his case it was the "precise" field he was exporting in events.

Vince

(yes, I still think events should be defined in a library or else a
user parsable file in userspace. Putting them in sysfs just complicates everything for no good reason)

Ok, let's improve documentation, how about the patch below?

I don't see anything special with config fields, why not expand the same functionality to other fields in perf_attr too? Your example for precise_ip shows this could be useful.

And again, a while ago there was the decision to use sysfs to specify events (I didn't like the approach too). Now, we must be able to setup *any* kind of event in that way, not just that ones that can be setup only by using config fields.

First of all, the change is backward compatible for any tool out. If not implemented, the parser throws an error and the event with the new format can not be setup via sysfs. If other tools do not use such events, no need to update anything.

The event parser became a bit complex already, thus it might be useful to split the code and put it in some library e.g. liblk or so. There are plans to do this.

-Robert

index 77f47ff..d8f348f 100644
@@ -1,20 +1,50 @@
-Where:		/sys/bus/event_source/devices/<dev>/format
+Where:		/sys/bus/event_source/devices/<pmu>/format/<name>
 Date:		January 2012
-Kernel Version: 3.3
+Kernel Version:	3.3
+		3.xx	(added attr<index>:<bits>)
 Contact:	Jiri Olsa <[email protected]>
-Description:
-		Attribute group to describe the magic bits that go into
-		perf_event_attr::config[012] for a particular pmu.
-		Each attribute of this group defines the 'hardware' bitmask
-		we want to export, so that userspace can deal with sane
-		name/value pairs.
-
-		Userspace must be prepared for the possibility that attributes
+
+Description:	Define formats for bit ranges in perf_event_attr
+
+		Specify a format to describe the magic bits that go
+		into struct perf_event_attr for a particular pmu.
+		Userspace can deal then with sane name/value pairs.
+
+		Bit range may be any bit mask of an u64 value (bits 0
+		to 63). The bit range may vary depending on the
+		system's endianess if the underlying field is an u32,
+		for example the format is for bp_type:
+
+			attr7:32-63 ... little endian
+			attr7:0-31  ... big endian
+
+		Userspace must be prepared for the possibility that formats
 		define overlapping bit ranges. For example:
-			attr1 = 'config:0-23'
-			attr2 = 'config:0-7'
-			attr3 = 'config:12-35'
 
-		Example: 'config1:1,6-10,44'
-		Defines contents of attribute that occupies bits 1,6-10,44 of
-		perf_event_attr::config1.
+			format1 = 'config:0-23'
+			format2 = 'config:0-7'
+			format3 = 'config:12-35'
+
+		Syntax			Description
+
+		config[012]*:<bits>.... Format that defines any
+					'hardware' bitmask in one of
+					the config fields.
+
+		attr<index>:<bits>..... Format that defines any
+					bitmask in any u64 field in
+					the perf_event_attr structure.
+					The index is a decimal number
+					specifying the u64 value to be
+					pointed to in perf_event_attr.
+					Index starts at zero.
+
+		Examples:
+
+		'config1:1,6-10,44'	Defines contents of attribute
+					that occupies bits 1,6-10,44
+					of perf_event_attr::config1.
+
+		'attr5:23'		Define the persistent event
+					flag (bit 23 of the attribute
+					flags)

From: Robert Richter <[email protected]>

Tracepoints have a unique attr.config value. But, this is not sufficient to support all event types. For this we need to generate unique event ids.

---
 kernel/events/persistent.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

index aca1e98..f23270b 100644
@@ -1,6 +1,7 @@
 #include <linux/slab.h>
 #include <linux/perf_event.h>
 #include <linux/ftrace_event.h>
+#include <linux/idr.h>
 
 #include "internal.h"
 

@@ -13,10 +14,37 @@ struct pevent {
 	int		id;
 };
 
+static struct idr event_idr;
+static struct mutex event_lock;
 static struct pmu persistent_pmu;
 static DEFINE_PER_CPU(struct list_head, pevents);
 static DEFINE_PER_CPU(struct mutex, pevents_lock);
 
+static inline struct pevent *find_event(int id)
+{
+	struct pevent *pevent;
+	rcu_read_lock();
+	pevent = idr_find(&event_idr, id);
+	rcu_read_lock();
+	return pevent;
+}
+
+static inline int get_event_id(struct pevent *pevent)
+{
+	int event_id;
+	mutex_lock(&event_lock);
+	event_id = idr_alloc(&event_idr, pevent, 1, INT_MAX, GFP_KERNEL);
+	mutex_unlock(&event_lock);
+	return event_id;
+}
+
+static inline void put_event_id(int id)
+{
+	mutex_lock(&event_lock);
+	idr_remove(&event_idr, id);
+	mutex_unlock(&event_lock);
+}
+
 /* Must be protected with pevents_lock. */
 static struct perf_event *__pevent_find(int cpu, int id)
 {

@@ -128,13 +156,16 @@ persistent_open(char *name, struct perf_event_attr *attr, int nr_pages)
 	struct pevent *pevent;
 	char id_buf[32];
 	int cpu;
-	int ret = 0;
+	int ret;
 
 	pevent = kzalloc(sizeof(*pevent), GFP_KERNEL);
 	if (!pevent)
 		return -ENOMEM;
 
-	pevent->id = attr->config;
+	ret = get_event_id(pevent);
+	if (ret < 0)
+		goto fail;
+	pevent->id = ret;
 
 	if (!name) {
 		snprintf(id_buf, sizeof(id_buf), "%d", pevent->id);

@@ -163,6 +194,9 @@ persistent_open(char *name, struct perf_event_attr *attr, int nr_pages)
 fail:
 	for_each_possible_cpu(cpu)
 		persistent_event_close(cpu, pevent);
+
+	if (pevent->id)
+		put_event_id(pevent->id);
 	kfree(pevent->name);
 	kfree(pevent);
 

@@ -306,6 +340,8 @@ void __init perf_register_persistent(void)
 {
 	int cpu;
 
+	idr_init(&event_idr);
+	mutex_init(&event_lock);
 	perf_pmu_register(&persistent_pmu, "persistent", PERF_TYPE_PERSISTENT);
 
 	for_each_possible_cpu(cpu) {

1.8.3.2

From: Robert Richter <[email protected]>

We need this later for proper event removal.

---
 kernel/events/persistent.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

index f23270b..70446ae 100644
@@ -9,6 +9,7 @@
 #define CPU_BUFFER_NR_PAGES	((512 * 1024) / PAGE_SIZE)
 
 struct pevent {
+	atomic_t	refcount;
 	struct perf_pmu_events_attr sysfs;
 	char		*name;
 	int		id;

@@ -130,6 +131,7 @@ static int persistent_event_open(int cpu, struct pevent *pevent,
 	if (ret)
 		goto fail;
 
+	atomic_inc(&pevent->refcount);
 	atomic_inc(&event->mmap_count);
 
 	/* All workie, enable event now */

@@ -144,8 +146,11 @@ static int persistent_event_open(int cpu, struct pevent *pevent,
 static void persistent_event_close(int cpu, struct pevent *pevent)
 {
 	struct perf_event *event = pevent_del(pevent, cpu);
-	if (event)
+	if (event) {
+		/* Safe, the caller holds &pevent->refcount too. */
+		atomic_dec(&pevent->refcount);
 		persistent_event_release(event);
+	}
 }
 
 static int pevent_sysfs_register(struct pevent *event);

@@ -162,6 +167,8 @@ persistent_open(char *name, struct perf_event_attr *attr, int nr_pages)
 	if (!pevent)
 		return -ENOMEM;
 
+	atomic_set(&pevent->refcount, 1);
+
 	ret = get_event_id(pevent);
 	if (ret < 0)
 		goto fail;

@@ -187,21 +194,21 @@ persistent_open(char *name, struct perf_event_attr *attr, int nr_pages)
 	}
 
 	ret = pevent_sysfs_register(pevent);
-	if (ret)
-		goto fail;
-
-	return 0;
+	if (!ret)
+		goto out;
 fail:
 	for_each_possible_cpu(cpu)
 		persistent_event_close(cpu, pevent);
 
-	if (pevent->id)
-		put_event_id(pevent->id);
-	kfree(pevent->name);
-	kfree(pevent);
-
 	pr_err("%s: Error adding persistent event: %d\n",
 		__func__, ret);
+out:
+	if (atomic_dec_and_test(&pevent->refcount)) {
+		if (pevent->id)
+			put_event_id(pevent->id);
+		kfree(pevent->name);
+		kfree(pevent);
+	}
 
 	return ret;
 }

1.8.3.2

From: Robert Richter <[email protected]>

There was a limitation of the total number of persistent events to be registered in sysfs due to the lack of dynamically list allocation. This patch implements memory reallocation in case an event is added or removed from the list.

While at this also implement pevent_sysfs_unregister() which we need later for proper event removal.

---
 kernel/events/persistent.c | 115 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 99 insertions(+), 16 deletions(-)

index 70446ae..a0ef6d4 100644
@@ -154,6 +154,7 @@ static void persistent_event_close(int cpu, struct pevent *pevent)
 }
 
 static int pevent_sysfs_register(struct pevent *event);
+static void pevent_sysfs_unregister(struct pevent *event);
 
 static int __maybe_unused
 persistent_open(char *name, struct perf_event_attr *attr, int nr_pages)

@@ -204,6 +205,7 @@ persistent_open(char *name, struct perf_event_attr *attr, int nr_pages)
 		__func__, ret);
 out:
 	if (atomic_dec_and_test(&pevent->refcount)) {
+		pevent_sysfs_unregister(pevent);
 		if (pevent->id)
 			put_event_id(pevent->id);
 		kfree(pevent->name);

@@ -273,13 +275,12 @@ static struct attribute_group persistent_format_group = {
 	.attrs = persistent_format_attrs,
 };
 
-#define MAX_EVENTS 16
-
-static struct attribute *pevents_attr[MAX_EVENTS + 1] = { };
+static struct mutex sysfs_lock;
+static int sysfs_nr_entries;
 
 static struct attribute_group pevents_group = {
 	.name = "events",
-	.attrs = pevents_attr,
+	.attrs = NULL,		/* dynamically allocated */
 };
 
 static const struct attribute_group *persistent_attr_groups[] = {

@@ -288,6 +289,7 @@ static const struct attribute_group *persistent_attr_groups[] = {
 	NULL,
 };
 #define EVENTS_GROUP_PTR	(&persistent_attr_groups[1])
+#define EVENTS_ATTRS_PTR	(&pevents_group.attrs)
 
 static ssize_t pevent_sysfs_show(struct device *dev,
 				struct device_attribute *__attr, char *page)

@@ -304,7 +306,9 @@ static int pevent_sysfs_register(struct pevent *pevent)
 	struct attribute *attr = &sysfs->attr.attr;
 	struct device *dev = persistent_pmu.dev;
 	const struct attribute_group **group = EVENTS_GROUP_PTR;
-	int idx;
+	struct attribute ***attrs_ptr = EVENTS_ATTRS_PTR;
+	struct attribute **attrs;
+	int ret = 0;
 
 	sysfs->id	= pevent->id;
 	sysfs->attr	= (struct device_attribute)

@@ -312,21 +316,99 @@ static int pevent_sysfs_register(struct pevent *pevent)
 	attr->name	= pevent->name;
 	sysfs_attr_init(attr);
 
-	/* add sysfs attr to events: */
-	for (idx = 0; idx < MAX_EVENTS; idx++) {
-		if (!cmpxchg(pevents_attr + idx, NULL, attr))
-			break;
+	mutex_lock(&sysfs_lock);
+
+	/*
+	 * Keep old list if no new one is available. Need this for
+	 * device_remove_attrs() if unregistering pmu.
+	 */
+	attrs = __krealloc(*attrs_ptr, (sysfs_nr_entries + 2) * sizeof(*attrs),
+			GFP_KERNEL);
+
+	if (!attrs) {
+		ret = -ENOMEM;
+		goto unlock;
 	}
 
-	if (idx >= MAX_EVENTS)
-		return -ENOSPC;
-	if (!idx)
+	attrs[sysfs_nr_entries++]	= attr;
+	attrs[sysfs_nr_entries]		= NULL;
+
+	if (!*group)
 		*group = &pevents_group;
+
+	if (!dev)
+		goto out;	/* sysfs not yet initialized */
+
+	if (sysfs_nr_entries == 1)
+		ret = sysfs_create_group(&dev->kobj, *group);
+	else
+		ret = sysfs_add_file_to_group(&dev->kobj, attr, (*group)->name);
+
+	if (ret) {
+		/* roll back */
+		sysfs_nr_entries--;
+		if (!sysfs_nr_entries)
+			*group = NULL;
+		if (*attrs_ptr != attrs)
+			kfree(attrs);
+		else
+			attrs[sysfs_nr_entries] = NULL;
+		goto unlock;
+	}
+out:
+	if (*attrs_ptr != attrs) {
+		kfree(*attrs_ptr);
+		*attrs_ptr = attrs;
+	}
+unlock:
+	mutex_unlock(&sysfs_lock);
+
+	return ret;
+}
+
+static void pevent_sysfs_unregister(struct pevent *pevent)
+{
+	struct attribute *attr = &pevent->sysfs.attr.attr;
+	struct device *dev = persistent_pmu.dev;
+	const struct attribute_group **group = EVENTS_GROUP_PTR;
+	struct attribute ***attrs_ptr = EVENTS_ATTRS_PTR;
+	struct attribute **attrs, **dest;
+
+	mutex_lock(&sysfs_lock);
+
+	for (dest = *attrs_ptr; *dest; dest++) {
+		if (*dest == attr)
+			break;
+	}
+
+	if (!*dest)
+		goto unlock;
+
+	sysfs_nr_entries--;
+
+	*dest = (*attrs_ptr)[sysfs_nr_entries];
+	(*attrs_ptr)[sysfs_nr_entries] = NULL;
+
 	if (!dev)
-		return 0;	/* sysfs not yet initialized */
-	if (idx)
-		return sysfs_add_file_to_group(&dev->kobj, attr, (*group)->name);
-	return sysfs_create_group(&persistent_pmu.dev->kobj, *group);
+		goto out;	/* sysfs not yet initialized */
+
+	if (!sysfs_nr_entries)
+		sysfs_remove_group(&dev->kobj, *group);
+	else
+		sysfs_remove_file_from_group(&dev->kobj, attr, (*group)->name);
+out:
+	if (!sysfs_nr_entries)
+		*group = NULL;
+
+	attrs = __krealloc(*attrs_ptr, (sysfs_nr_entries + 1) * sizeof(*attrs),
+			GFP_KERNEL);
+
+	if (!attrs && *attrs_ptr != attrs) {
+		kfree(*attrs_ptr);
+		*attrs_ptr = attrs;
+	}
+unlock:
+	mutex_unlock(&sysfs_lock);
 }
 
 static int persistent_pmu_init(struct perf_event *event)

@@ -349,6 +431,7 @@ void __init perf_register_persistent(void)
 
 	idr_init(&event_idr);
 	mutex_init(&event_lock);
+	mutex_init(&sysfs_lock);
 	perf_pmu_register(&persistent_pmu, "persistent", PERF_TYPE_PERSISTENT);
 
 	for_each_possible_cpu(cpu) {

1.8.3.2

From: Robert Richter <[email protected]>

Implementing ioctl functions to control persistent events. There are functions to detach or attach an event to or from a process. The PERF_EVENT_IOC_DETACH ioctl call makes an event persistent. After closing the event's fd it runs then in the background of the system without the need of a controlling process. The perf_event_open() syscall can be used to reopen the event by any process. The PERF_EVENT_IOC_ATTACH ioctl attaches the event again so that it is removed after closing the event's fd.

This is for Linux man-pages:

type ...

PERF_TYPE_PERSISTENT (Since Linux 3.xx)

This indicates a persistent event. There is a unique identifier for each persistent event that needs to be specified in the event's attribute config field. Persistent events are listed under:

/sys/bus/event_source/devices/persistent/

...
persistent : 41, /* always-on event */
...

persistent: (Since Linux 3.xx)

Put event into persistent state after opening. After closing the event's fd the event is persistent in the system and continues to run.

perf_event ioctl calls

PERF_EVENT_IOC_DETACH (Since Linux 3.xx)

Detach the event specified by the file descriptor from the process and make it persistent in the system. After closing the fd the event will continue to run. An unique identifier for the persistent event is returned or an error otherwise. The following allows to connect to the event again:

pe.type = PERF_TYPE_PERSISTENT;
pe.config = <pevent_id>;
...
fd = perf_event_open(...);

The event must be reopened on the same cpu.

PERF_EVENT_IOC_ATTACH (Since Linux 3.xx)

Attach the event specified by the file descriptor to the current process. The event is no longer persistent in the system and will be removed after all users disconnected from the event. Thus, if there are no other users the event will be closed too after closing its file descriptor, the event then no longer exists.

---
 include/uapi/linux/perf_event.h |   2 +
 kernel/events/core.c            |   6 ++
 kernel/events/internal.h        |   2 +
 kernel/events/persistent.c      | 178 +++++++++++++++++++++++++++++++++-------
 4 files changed, 160 insertions(+), 28 deletions(-)

index 2b84b97..82a8244 100644
@@ -324,6 +324,8 @@ struct perf_event_attr {
 #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_DETACH		_IO ('$', 8)
+#define PERF_EVENT_IOC_ATTACH		_IO ('$', 9)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,

index d9d6e67..8d5c6e3 100644
@@ -3622,6 +3622,12 @@ static long perf_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case PERF_EVENT_IOC_SET_FILTER:
 		return perf_event_set_filter(event, (void __user *)arg);
 
+	case PERF_EVENT_IOC_DETACH:
+		return perf_event_detach(event);
+
+	case PERF_EVENT_IOC_ATTACH:
+		return perf_event_attach(event);
+
 	default:
 		return -ENOTTY;
 	}

index 94c3f73..f9bc15f 100644
@@ -195,5 +195,7 @@ extern void perf_free_rb(struct perf_event *event);
 extern int perf_get_fd(struct perf_event *event);
 extern int perf_get_persistent_event_fd(int cpu, int id);
 extern void __init perf_register_persistent(void);
+extern int perf_event_detach(struct perf_event *event);
+extern int perf_event_attach(struct perf_event *event);
 
 #endif /* _KERNEL_EVENTS_INTERNAL_H */

1.8.3.2

I think these aren't very good names for the ioctls. Maybe something like
PERF_EVENT_IOC_MAKE_PERSISTENT
PERF_EVENT_IOC_UNPERSIST
I know that last one's not a real word but I can't think of what the proper term would be. Maybe
PERF_EVENT_IOC_RELEASE_PERSISTENT PERF_EVENT_IOC_RECLAIM_PERSISTENT

Thanks, though you're missing out by not learning all about troff formatting.

Wait, so the first time you create a persistent event you do *not* set type PERF_TYPE_PERSISTENT? You only do that if you're "attaching" to an exisiting event? You might want to clarify that.

will there be some sort of tool that will let you kill runaway persistent events? Or will you have to manually perf_event_open() / iotcl() them by hand somehow?

You might want to re-order things so it's clear you get the unique ID at ioctl time and not after the close happens.

Vince

"aren't very good names" is not really an argument I can work with. Why not? What if you want to attach/detach to events but not be persistent. Which also begs the question how long are we persistent? The whole system runtime or until the user decides to detach.

So ATTACH/DETACH in the sense of attaching processes to events for an arbitrary amount of time *and* *not* for the duration of the tracee as we do it currently implicitly, is much more generic wrt usage than specifying that specific persistent case.

Thanks.

Ok, for clarification, my intention was to say something like 'detach event from the current process controlling it', or 'attach the event to the current process that holds the fd'.

Whatever term is the best for this ioctls, I am fine with it. The above terms look a bit long.

The problem with detach/attach is more that it's actually more logically to attach first and afterwards detach. This is not the case here, it's vise versa.

-Robert

Maybe this?

PERF_EVENT_IOC_PERSIST
PERF_EVENT_IOC_UNPERSIST

-Robert

No, ATTACH/DETACH actually describes what you do with the fds and is most generic. "PERSIST*" is a special use case of attaching/detaching events.

Thanks.

I agree with what Robert said elsewhere in this thread:
"The problem with detach/attach is more that it's actually more
logically to attach first and afterwards detach. This is not the case here, it's vise versa."

My main confusion is that with some other performance tools, such as PAPI, "attach" specifically means to take an event and attach it to a process (much like the -p option to strace or gdb) or a cpu. Now perf_event handles this differently (you do that at open time) but I still think it gets things backwards.

Since ATTACH is usually a transitive verb in English I'd think the name should specify what two things are being attached.

If I had not read the man-page fragment and saw a
result=ioctl(fd,PERF_EVENT_IOC_ATTACH,0);
I'd have no clue what it was doing (attach? attach to what?) wheras if I saw
result=ioctl(fd,PERF_EVENT_IOC_UNPERSIST,0);
it's a little more clearer and also indicates that the ioctl is only valid if you're dealing with a persistent event.

Vince

Maybe this makes it more understandable for you but this is beside the point.

The main and the most important idea here is that we want to attach/detach file descriptors to perf events and the resources (buffers, etc) associated with them so that those events can be made independent from processes.

But I have to say the reversed thing above does sound confusing, now that I'm looking at the code. Actually, at the time we discussed this, my idea was to do it like this:

1. we open a perf event and get its file descriptor
2. ioctl ATTACH to it so that it is attached to the process.

... do some tracing and collecting and fiddling...

3. ioctl DETACH from it so that it is "forked in the background" so to speak, very similar to a background job in the shell.

4. The rest of the code continues and deallocates the event *BUT* (and this is the key thing!) the counter/tracepoint remains operational in the kernel, running all the time.

5. Now, after a certain point, you come back and ioctl ATTACH to this already opened event and read/collect its buffers again.

And here's the deal - if you don't DETACH from the event at step 3, it gets destroyed on process exit, i.e. what the current perf behavior is.

Robert, I think the above is more straight-forward and intuitive, no?

Thanks.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Understandability doesn't matter?

Would it make sense to actually fork a kernel thread that "owns" the event?

The way it is now events can "get loose" if either the user forgets about them or the tool that opened them crashes, and it's impossible to kill these events with normal tools. You possibly wouldn't even know one was running (except you'd have one fewer counter to work with) unless you poked around under /sys.

Vince

Actually, the idea is to be able to reattach and control that event with perf tool too, in addition to some specialized daemon or whatever. So whatever else "lost" it, using perf tool you should be able to get it back.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

I agree with Vince on this, the naming should be intuitive. Esp. since 'attach' is used in tracing in different context, we should avoid using the term here for less confusion.

I got another idea for this, what about UNCLAIM and CLAIM? It is exactly, what it is. A process unclaims an event telling it doesn't care anymore. Another process comes and claims the event, meaning the process wants the event no longer to be shared with others and being released after closing.

You don't need to attach to a persistent event, you can just use the event with perf_event_open, mmap, close.

With 'detach' we move the event into the 'background' so that it is still available after opening.

There is no need for a kernel thread, there is nothing to do. We just increase the refcount so that the event is not destroyed.

As boris said, there could be some sort of 'kill' tool for events. That's what we need sysfs for, it tells which events are running.

The event remains operational esp. after closing it or killing/ terminating the process owning it.

-Robert

This still doesn't pan out because with claiming the event, you state that the event is *owned* by this process but with persistent events we want to be able to state that they can have multiple users and thus multiple buffer consumers, concurrently.

Ok, maybe ATTACH/DETACH is not the perfect naming for this after all. Maybe when we want to state the fact that the event is going to continue existing after closing the buffer consumer, we want to do ioctl(event, DONT_DESTROY) and when we want to actually get rid of it, one of the process does ioctl(event, DESTROY).

Which reminds me, what do we do when one process destroys the event but there are other consumers accessing it concurrently? Refcounting?

Thanks.

Regards/Gruss,
Boris.

Ok, starting with your question at the end first since it might explain the following better.

If the event is no longer persistent and if there are multiple users, the event remains open and running until the all mmap's are munmap'ed and the last fd is closed. After that it is released. This is done with refcounts and already implemented.

We still have multiple users after 'claiming' the event. The only thing is that the event will be destroyed after all users went away. A process (that claimed the event) was responsible for this.

I still prefer claim/unclaim. ;)

See above.

-Robert

Ok, if the "claimer" process which owns the event dies for some reason, then I'm guessing some other event can claim it on its own, i.e. processes can claim events among each other...

Oh well, whatever, as long as it not misleading :-)

Regards/Gruss,
Boris.

Thanks for documenting perf_event_open(), it's a great help. The troff thing I may learn in case I write a patch for man-pages. ;)

You just *open* an existing persistent event and may access buffers with mmap(). After opening it you also may attach the event to the process holding the fd with the ioctl. Then, the event is no longer persistent and removed after all users finished using the event. Without doing the attach-ioctl the event stays persistent in the system.

The instance opening the event should also be responsible for removing it. But there could be a perf tool for controlling persistent events (create, list, remove, etc).

Ah, yes, indeed:

Detach the event specified by the file descriptor from the process and make it persistent in the system. An unique identifier for the persistent event is returned or an error otherwise. After closing the fd the event will continue to run. The following allows to connect to the event again:

Thanks for review, Vince.

-Robert

Updated description below.

-Robert

Author: Robert Richter <[email protected]>
Date:   Tue Aug 13 11:22:22 2013 +0200

[RFC] perf, persistent: ioctl functions to control persistency

Implementing ioctl functions to control persistent events. There are functions to detach or attach an event to or from a process. The PERF_EVENT_IOC_DETACH ioctl call makes an event persistent. After closing the event's fd it runs then in the background of the system without the need of a controlling process. The perf_event_open() syscall can be used to reopen the event by any process. The PERF_EVENT_IOC_ATTACH ioctl attaches the event again so that it is removed after closing the event's fd.

This is for Linux man-pages:

type ...

PERF_TYPE_PERSISTENT (Since Linux 3.xx)

Open a persistent event that is already running in the background of the system. There is a unique identifier for each persistent event that needs to be specified in the event's attribute config field. Persistent events are listed under:

/sys/bus/event_source/devices/persistent/

See PERF_EVENT_IOC_DETACH how to create a persistent event. The instance creating such an event should also be responsible for removing it.

...
persistent : 41, /* always-on event */
...

persistent: (Since Linux 3.xx)

Put event into persistent state after opening. After closing the event's fd the event is persistent in the system and continues to run.

perf_event ioctl calls

PERF_EVENT_IOC_DETACH (Since Linux 3.xx)

Any event that was opened with the perf_event_open() syscall may become a persistent event. This is done by detaching the event from the controlling process that holds the event's file descriptor. This ioctl can be used for doing this. After detaching it, the event is persistent in the system. An unique identifier for the persistent event is returned or an error otherwise. After closing the fd the event will continue to run. The following allows to connect to the event again:

pe.type = PERF_TYPE_PERSISTENT; pe.config = <pevent_id>;
...
fd = perf_event_open(...);

The event must be reopened on the same cpu.

PERF_EVENT_IOC_ATTACH (Since Linux 3.xx)

Attach the event specified by the file descriptor to the current process. The event is no longer persistent in the system and will be removed after all users disconnected from the event. Thus, if there are no other users the event will be closed too after closing its file descriptor, the event then no longer exists.

Yep,

this text is very nicely written and should be in a README somewhere. :-)

Yeah, we probably want to abstract this a step further by allowing to attach/detach fds to/from events. Then "persistent" is only one incarnation of us always detaching from the event during its lifetime.

If we close an event while it is attached, it gets destroyed - i.e., current functionality, etc. See the other thread.

But we probably need more input on this from people. Ingo, Peter?

Thanks.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Yeah, will put it under tools/perf/Documentation/...

I don't know what you mean here exactly, please explain.

Yes, that would be great.

Thanks,

-Robert

Basically that detaching an event shouldn't make it persistent explicitly - it simply continues running in the background. When we reattach to it and die with the event attached, then it gets destroyed too.

Which means, we can have arbitrary life periods of events, persistency being only a special case of it.

IOW, as long as an event is detached in the background, it counts. When something attaches to it and that something exits, the event gets destroyed too, as part of the process teardown.

And this is probably the most generic way to look at it.

Regards/Gruss,
Boris.

Hi,

Now that the feature is resurfacing, I would like to take over the task for persistent events.
What is the status of the series, is it close to acceptance?

AFAICS the patch RFC 12/12 was under discussion, mainly about the naming of the ioctls (latest e-mail is in favor of CLAIM/UNCLAIM).

Any thoughts?

Let me rebase the code against the latest mainline and re-start the discussion.

Thread: https://lkml.org/lkml/2013/8/22/306
RFC 12/12: https://lkml.org/lkml/2013/8/27/264

Regards,
Jean

That's nice, cool. However, before this thing goes upstream, Ingo wanted to have also a user for the events, i.e. persistent events integrated in perf tool. For that, perf tool functionality has to be made generic enough and thus usable for other tools.

I have started doing that but other stuff preempted me, as it happens :(. Here's how a splitup would look like, from an older email from Ingo. In any case, this would need further discussion but I think it is a good start.

I'm willing to help out as time permits and I'd venture a guess that Robert would do that too, so please CC us your submissions.

Thanks!

--

Date: Tue, 26 Nov 2013 19:17:45 +0100
From: Ingo Molnar <[email protected]>
To: Borislav Petkov <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>, LKML
<[email protected]>, Borislav Petkov <[email protected]>, Jiri Olsa <[email protected]>, Peter Zijlstra <[email protected]>, Robert Richter <[email protected]>
Subject: Re: [PATCH] perf: Move fs.* to generic lib/lk/
Message-ID: <[email protected]>
User-Agent: Mutt/1.5.21 (2010-09-15)
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=utf-8
Status: RO
X-Status: A

It's a single line added to the Makefile, the moment a .h is used for the first time. That's not any appreciable overhead.

This would also allow us to farm out most of tools/perf/util/ into tools/lib/, without any noticeable changes in build performance or build dependencies. Down the line it would (hopefully) result in code improvements to these infrastructure bits, sourced from different tools.

Well, the natural evolution of interfaces ended up with such a split up:

comet:~/tip/tools/perf> ls util/*.h

util/annotate.h   util/hist.h           util/strbuf.h
util/build-id.h   util/intlist.h        util/strfilter.h
util/cache.h      util/levenshtein.h    util/strlist.h
util/callchain.h  util/machine.h        util/svghelper.h
util/cgroup.h     util/map.h            util/symbol.h
util/color.h      util/parse-events.h   util/target.h
util/comm.h       util/parse-options.h  util/thread.h
util/cpumap.h     util/perf_regs.h      util/thread_map.h
util/data.h       util/pmu.h            util/tool.h
util/debug.h      util/probe-event.h    util/top.h
util/dso.h        util/probe-finder.h   util/trace-event.h
util/dwarf-aux.h  util/pstack.h         util/types.h
util/event.h      util/quote.h          util/unwind.h
util/evlist.h     util/rblist.h         util/util.h
util/evsel.h      util/run-command.h    util/values.h
util/exec_cmd.h   util/session.h        util/vdso.h
util/fs.h         util/sigchain.h       util/xyarray.h
util/header.h     util/sort.h
util/help.h       util/stat.h

If we want additional structure to it then it should be done via the namespace, not by forcing them into bigger .a's. So this kind of extra structure makes sense:

api/types/rbtree.h
api/types/strbuf.h
api/formats/dwarf/unwind.h
api/kernel/pmu.h
api/kernel/cgroup.h
api/kernel/debugfs.h

But stuffing them into types.a, formats.a, kernel.a, not so much.

glibc being such a catch-all library is:

- partly a historic artifact caused by other constraints that don't affect our tooling landscape here

- partly a political artifact caused by thinking that does not affect our tooling landscape

- partly a technological mistake.

There's no need for us to repeat that, at least at this stage.

You still need to figure out the .h file - at that point, when you are using it for the first time in your tool project, you add the .c file to the Makefile - it's not hard and there are real advantages.

It does not look that nice and clean once I consider all the nice helpers that exist in util/*.[ch] - and which we'd like to share as well.

Thanks,

Ingo