Persistent events

From: Borislav Petkov <[email protected]>

This generic interface is supposed to be used to add files to (debugfs)/tracing/events/<subsys>/<event_name>/ which are supposed to export additional events functionality to users.

For example, the file descriptor of a persistent event can be exported like this in the 'pers' node:

/mnt/dbg/tracing/events/mce/mce_record/

|-- enable
|-- filter
|-- format
|-- id

`-- persistent

So that userspace tools can read it out and map it directly.

Cc Peter Zijlstra <[email protected]>

Cc Steven Rostedt <[email protected]>

Signed-off-by Borislav Petkov <[email protected]>

---
 include/linux/ftrace_event.h |  3 +++
 kernel/trace/trace.c         |  4 ++++
 kernel/trace/trace.h         |  1 +
 kernel/trace/trace_events.c  | 35 +++++++++++++++++++++++++++++++++++
 4 files changed, 43 insertions(+)

include/linux/ftrace_event.h +3 -0

index 642928cf57b4..f4f283954cbf 100644
@@ -265,6 +265,9 @@ extern int trace_define_field(struct ftrace_event_call *call, const char *type,
 			      int is_signed, int filter_type);
 extern int trace_add_event_call(struct ftrace_event_call *call);
 extern void trace_remove_event_call(struct ftrace_event_call *call);
+extern struct dentry *
+trace_add_file(const char *dir_name, const char *fname, umode_t mode,
+		void *data, const struct file_operations *fops);
 
 #define is_signed_type(type)	(((type)(-1)) < 0)
 

kernel/trace/trace.c +4 -0

index 5c38c81496ce..e45479572328 100644
@@ -4641,6 +4641,10 @@ struct dentry *trace_create_file(const char *name,
 	return ret;
 }
 
+void trace_remove_file(struct dentry *d)
+{
+	return debugfs_remove(d);
+}
 
 static struct dentry *trace_options_init_dentry(void)
 {

kernel/trace/trace.h +1 -0

index 55e1f7f0db12..fd639f45c425 100644
@@ -338,6 +338,7 @@ struct dentry *trace_create_file(const char *name,
 				 struct dentry *parent,
 				 void *data,
 				 const struct file_operations *fops);
+void trace_remove_file(struct dentry *d);
 
 struct dentry *tracing_init_dentry(void);
 

kernel/trace/trace_events.c +35 -0

index 29111da1d100..5eda4f584380 100644
@@ -1199,6 +1199,41 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
 	return 0;
 }
 
+static struct dentry *__trace_add_file(struct ftrace_event_call *call,
+				       const char *fname, umode_t mode,
+				       void *data,
+				       const struct file_operations *fops)
+{
+	struct dentry *ret;
+
+	mutex_lock(&event_mutex);
+	ret = trace_create_file(fname, mode, call->dir, data, fops);
+	mutex_unlock(&event_mutex);
+
+	return ret;
+}
+
+/*
+ * Assumptions:
+ * - event debugfs dir is already initialized
+ * - trace event is not declared in a module
+ */
+struct dentry *trace_add_file(const char *dir_name, const char *fname,
+			      umode_t mode, void *data,
+			      const struct file_operations *fops)
+{
+	struct ftrace_event_call *call;
+
+	if (!strlen(fname))
+		return NULL;
+
+	list_for_each_entry(call, &ftrace_events, list) {
+		if (!strncmp(call->name, dir_name, strlen(dir_name)))
+			return __trace_add_file(call, fname, mode, data, fops);
+	}
+	return NULL;
+}
+
 static int
 __trace_add_event_call(struct ftrace_event_call *call, struct module *mod,
 		       const struct file_operations *id,

Quoted 6 lines

/mnt/dbg/tracing/events/mce/mce_record/

|-- enable
|-- filter
|-- format
|-- id

`-- persistent

This looks rather hacky :-/

Can't this be added to the perf syscall, and have the user query the event instead?

Note, this code will change drastically soon, and this change will break. I'll talk more about what will change at plumbers.

kernel/events/core.c static struct ring_buffer *ring_buffer_get(struct perf_event *event) · line 3519

Quoted 6 lines · Borislav Petkov

kernel/events/core.c +1 -1

-static void ring_buffer_put(struct ring_buffer *rb)
+void ring_buffer_put(struct ring_buffer *rb)
 {
 	struct perf_event *event, *n;
 	unsigned long flags;

I have to say that it is really unfortunate that we have two ring buffers in the kernel, called struct ring_buffer. One being global and one being local to events. And now we are exporting the name "ring_buffer_*" too! This is going to confuse the hell out of people.

Perhaps this should have been called perf_buffer, as that is what it's used for.

kernel/events/core.c static struct ring_buffer *ring_buffer_get(struct perf_event *event) · line 3519

Quoted 6 lines · Borislav Petkov

kernel/events/core.c +1 -1

-static void ring_buffer_put(struct ring_buffer *rb)
+void ring_buffer_put(struct ring_buffer *rb)
 {
 	struct perf_event *event, *n;
 	unsigned long flags;

I have to say that it is really unfortunate that we have two ring buffers in the kernel, called struct ring_buffer. One being global and one being local to events. And now we are exporting the name "ring_buffer_*" too! This is going to confuse the hell out of people.

Perhaps this should have been called perf_buffer, as that is what it's used for.

off and on I get some free time to work on that, here's the latest incarnation. It contains review feedback from the earlier round.

Quoted 2 lines · On Thu, 16 Aug 2012 19:45:19 +0200 Borislav Petkov <[email protected]> wrote:

off and on I get some free time to work on that, here's the latest incarnation. It contains review feedback from the earlier round.

Can I add one small request for the next round? I've looked through the patches and the code a bit, and I still have absolutely no clue of what a "persistent event" is or why I might want one. Now, admittedly, I'm slower than most, but it still might not hurt to add some overall description of what this is for...?

Hi Jon,

Quoted 11 lines · Jonathan Corbet

Quoted 2 lines · On Thu, 16 Aug 2012 19:45:19 +0200 Borislav Petkov <[email protected]> wrote:

off and on I get some free time to work on that, here's the latest incarnation. It contains review feedback from the earlier round.

Can I add one small request for the next round? I've looked through the patches and the code a bit, and I still have absolutely no clue of what a "persistent event" is or why I might want one. Now, admittedly, I'm slower than most, but it still might not hurt to add some overall description of what this is for...?

yeah, maybe the naming is not the most fitting one but here's the basic idea:

Normally, you enable perf events for a duration of time where you trace your workload and then you disable them. In contrast, persistent events are perf events which you enable at system boot and they remain enabled
- thus persistent - during the whole system lifetime.

The machinery logs all events and carries them out to userspace. Much like the blackbox of a plane which records system events during the whole flight.

And the usecase here is that we want the machine check code to log machine checks during the whole system lifetime using the perf/trace events infrastructure. And this happens regardless of whether there's a userspace consumer of the logged data. Normally, we'll have one though :).

MCA and the trace_mce_record() tracepoint is thus is the first user of the persistent events but there might be others.

Please follow this thread:
http://marc.info/?l=linux-kernel&m=133234057614885 for some more thoughts on the idea.

Of course, this whole thing is basically a rough draft and we're still hammering out the details as we go.

BTW, we already have a persistent buffering in the kernel. It's used by ftrace. What about having perf use that buffering for persistent events? Or is there some other issues about using it.

I'm currently working on having perf read ftrace data, so in the near future, I plan on having some RFC patches to have perf reading from this buffer anyway.

Quoted 3 lines · Steven Rostedt

BTW, we already have a persistent buffering in the kernel. It's used by ftrace. What about having perf use that buffering for persistent events? Or is there some other issues about using it.

Not that I know of.

I coded this with the perf ring buffer now. If only there was one ring buffer in the kernel.. /me ducks and hides.

Ok, with perf I can read out the events programmatically by mmaping the per-CPU buffer with perf_mmap. How do you do that in ftrace? Any code pointers I can stare at?

Quoted 4 lines

I'm currently working on having perf read ftrace data, so in the near future, I plan on having some RFC patches to have perf reading from this buffer anyway.

Are you saying the ftrace buffer would be mmappable too now?

Btw, I wanted to hear your opinion on patch 1/4 since it touches ftrace/trace_events code. Can you please look at it and tell me if its ok?

Do these need to be mmapped, or are they copied into another location (disk or network)? I had code to allow the ftrace ring buffers to be mmapped, but stopped that work a while ago.

Quoted 5 lines

Quoted 3 lines

I'm currently working on having perf read ftrace data, so in the near future, I plan on having some RFC patches to have perf reading from this buffer anyway.

Are you saying the ftrace buffer would be mmappable too now?

No, the ftrace buffer was optimized for the splice command.

I would have perf do what trace-cmd does. That is, write directly into a file as it reads it, without ever needing the info to come into userspace.

I wasn't planning on replacing perf buffers, I was just planning on giving perf a tracing boost.

Another alternative, which would be a bit slower, would be to copy the pages, either into userspace (as a read) or into a location to mmap the buffers.

I could also continue the work on allowing ftrace buffers to be mmapped. But if the best thing to do is just use the perf buffers, then we could just stick with that.

Quoted 4 lines

Btw, I wanted to hear your opinion on patch 1/4 since it touches ftrace/trace_events code. Can you please look at it and tell me if its ok?

I'll take a look.

Quoted 3 lines · Steven Rostedt

Do these need to be mmapped, or are they copied into another location (disk or network)? I had code to allow the ftrace ring buffers to be mmapped, but stopped that work a while ago.

Well, my angle here is that since those are persistent, i.e. always-on events, they should be as low overhead as possible. So mmaping the buffer is probably that since it doesn't involve any data shuffling to and fro...

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

I'm currently working on having perf read ftrace data, so in the near future, I plan on having some RFC patches to have perf reading from this buffer anyway.

Are you saying the ftrace buffer would be mmappable too now?

No, the ftrace buffer was optimized for the splice command.

Ah, the splice thing moving data between two fds.

Quoted 4 lines

I would have perf do what trace-cmd does. That is, write directly into a file as it reads it, without ever needing the info to come into userspace.

Is that a real file?

Because what I was doing in the RAS daemon is mmap the buffer, read out the logged events in userspace and work on them.

Having them go to a file which I need to open in userspace adds one more step and I don't know how that would pan out in a critical situation of you getting a machine check where every insn counts. And besides, in a MCE situation you cannot be sure that the events would actually go to file... So my current impression is that reading them direct from memory is the fastest we can do. But this is only me and I have been wrong in the past. Lotsa times :).

Quoted 2 lines

I wasn't planning on replacing perf buffers, I was just planning on giving perf a tracing boost.

Cool.

Quoted 3 lines

Another alternative, which would be a bit slower, would be to copy the pages, either into userspace (as a read) or into a location to mmap the buffers.

Yeah, copying is not a good idea, IMHO, due to the said above.

Quoted 4 lines

I could also continue the work on allowing ftrace buffers to be mmapped. But if the best thing to do is just use the perf buffers, then we could just stick with that.

The easiest for now, I'd say, since it is already there.

If it wasn't for sparc, we could have done this with the ftrace buffers, but sparc has some crazy requirement that physical pages can only be mapped at certain locations, and you can't just take a bunch of random pages and map them at the same location.

off and on I get some free time to work on that, here's the latest incarnation. It contains review feedback from the earlier round.

Patch 1/4 adds a trace_add_file() interface which adds an additional file to debugfs, in this case the "persistent" file which contains the normal perf file descriptor sys_perf_event_open gives to the perf tool.

IOW, one gets:

/mnt/dbg/tracing/events/mce/mce_record/

|-- enable
|-- filter
|-- format
|-- id

`-- persistent1

0 directories, 5 files

[ 1 is the CPU number so sticking all per-CPU descriptors in this directory could get a little cluttered and ugly so I'll have to think about that a bit more. ]

3/4 is the meat which adds <kernel/events/persistent.c> and 4/4 shows how one can init a persistent event on a CPU.

What remains is adding code which can enable events on boot from the kernel cmdline and more testing.

As always, comments and suggestions are appreciated.

Good progress there, there's still a few things though:

- the point also raised by Steven, I'm pretty sure that the placing of the debugfs files unfortunate. I would much rather see something like /debug/perf/persistent/$foo, also dropping your perf_event_desc::dir_name.

- I would make perf_add_persistent_on_cpu() static and create something like perf_add_persistent() which iterates all CPUs and
creates: "%s-%04d", perf_event_desc::fname, cpu.

This needs a little extra for cpu-hotplug, not sure what to do there.

- related to the first point, by not tying them to actual events you can create a persistent 'event' that contains multiple events. Its quite possible to create multiple kernel events and use the equivalent of PERF_EVENT_IOC_SET_OUTPUT on them to the exposed FD.

- It might be good to provide means of changing the persistent event's buffer size, or maybe even 'destroy' persistent buffers.

Quoted 6 lines · Peter Zijlstra

Good progress there, there's still a few things though:

- the point also raised by Steven, I'm pretty sure that the placing of the debugfs files unfortunate. I would much rather see something like /debug/perf/persistent/$foo, also dropping your perf_event_desc::dir_name.

Ok, how do we want to do the per-CPU layout there? Like this:

/debug/perf/persistent/mce_record0
/debug/perf/persistent/mce_record1
...

or rather

/debug/perf/persistent/cpu0/mce_record
/debug/perf/persistent/cpu1/mce_record

?

Definitely the second one. The first one is just ugly. The second is more in line to the tracing directories too:

/debug/tracing/per_cpu/cpu0/...

Quoted 4 lines · Steven Rostedt

Definitely the second one. The first one is just ugly. The second is more in line to the tracing directories too:

/debug/tracing/per_cpu/cpu0/...

Maybe I can reuse your code and add

/debug/tracing/per_cpu/cpu0/persistent/<event>

?

It is basically debugfs handling, right? Or are you reworking that too?