| Message ID | 1410522513-1045-3-git-send-email-pawel.moll@arm.com |
|---|---|
| State | New |
| Headers | show |
Hi Pawel, On 09/12/2014 07:48 AM, Pawel Moll wrote: > This patch adds a PERF_COUNT_SW_MARKER event type, which > can be requested by user and a PERF_EVENT_IOC_MARKER > ioctl command which will inject an event of said type into > the perf buffer. The ioctl can take a zero-terminated > string argument, similar to tracing_marker in ftrace, > which will be kept in the "raw" field of the sample. > > The main use case for this is synchronisation of > performance data generated in user space with the perf > stream coming from the kernel. For example, the marker > can be inserted by a JIT engine after it generated > portion of the code, but before the code is executed > for the first time, allowing the post-processor to > pick the correct debugging information. Other example > is a system profiling tool taking data from other > sources than just perf, which generates a marker > at the beginning at at the end of the session > (also possibly periodically during the session) to > synchronise kernel timestamps with clock values > obtained in userspace (gtod or raw_monotonic). > @@ -5960,6 +5965,44 @@ static struct pmu perf_swevent = { > .event_idx = perf_swevent_event_idx, > }; > > +static int perf_sw_event_marker(struct perf_event *event, char __user *arg) > +{ > + struct perf_sample_data data; > + struct pt_regs *regs = current_pt_regs(); > + struct perf_raw_record raw = { 0, }; > + > + if (!static_key_false(&perf_swevent_enabled[PERF_COUNT_SW_MARKER])) > + return 0; > + > + perf_sample_data_init(&data, 0, 0); > + > + if (arg) { > + long len = strnlen_user(arg, PAGE_SIZE); Just to ask the dumb questions in case the answers I've come up with are wrong: What is PAGE_SIZE on an arm64 kernel? How does userspace know? Thanks, Christopher
On Fri, 2014-09-12 at 13:43 +0100, Christopher Covington wrote: > Just to ask the dumb questions in case the answers I've come up with are > wrong: What is PAGE_SIZE on an arm64 kernel? It's either 4 or 64k, depending on CONFIG_ARM64_64K_PAGES. > How does userspace know? > #include <unistd.h> #include <stdio.h> int main(void) { printf("%ld\n", sysconf(_SC_PAGESIZE)); return 0; } Now a word of explanation. The PAGE_SIZE limitation was shamelessly stolen from perf_event_set_filter() (so PERF_EVENT_IOC_SET_FILTER) as an attempt to address a problem of passing a zero-terminated string from userspace. Simply speaking - there must be some limitation, and a page size seem as good as any other. I have strong doubts about this myself, so all alternative ideas are more than welcome. As I mentioned in the cover letter, maybe this simply shouldn't be a string? I made it like this to mimic trace_marker, but maybe an integer value + some kind of a dictionary in userspace is a better approach? I belive that ftrace's maker is taking a string, because it's: 1. natural for its interface and 2. anyone (sort of) can write to it, so it's hard to assume anything. In this case the user "owns" the perf data, so he could handle int<->whatever-else relation table... Pawel > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Em Fri, Sep 12, 2014 at 01:57:52PM +0100, Pawel Moll escreveu: > On Fri, 2014-09-12 at 13:43 +0100, Christopher Covington wrote: > > Just to ask the dumb questions in case the answers I've come up with are > > wrong: What is PAGE_SIZE on an arm64 kernel? > It's either 4 or 64k, depending on CONFIG_ARM64_64K_PAGES. > > How does userspace know? > #include <unistd.h> > #include <stdio.h> > int main(void) > { > printf("%ld\n", sysconf(_SC_PAGESIZE)); > return 0; > } > Now a word of explanation. The PAGE_SIZE limitation was shamelessly > stolen from perf_event_set_filter() (so PERF_EVENT_IOC_SET_FILTER) as an > attempt to address a problem of passing a zero-terminated string from > userspace. Simply speaking - there must be some limitation, and a page > size seem as good as any other. I have strong doubts about this myself, > so all alternative ideas are more than welcome. > As I mentioned in the cover letter, maybe this simply shouldn't be a > string? I made it like this to mimic trace_marker, but maybe an integer > value + some kind of a dictionary in userspace is a better approach? I > belive that ftrace's maker is taking a string, because it's: 1. natural > for its interface and 2. anyone (sort of) can write to it, so it's hard > to assume anything. In this case the user "owns" the perf data, so he > could handle int<->whatever-else relation table... Perhaps both? I.e. an u64 followed from a string, if the u64 is zero, then there is a string right after it? - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Fri, 2014-09-12 at 14:49 +0100, Arnaldo Carvalho de Melo wrote: > Perhaps both? I.e. an u64 followed from a string, if the u64 is zero, > then there is a string right after it? How would this look like in userspace? Something like this? 8<---- struct perf_event_marker { uint64_t value; char *string; } arg; arg.value = 0x1234; /* or */ arg.value = 0; arg.string = "abcd"; ioctl(fd, PERF_EVENT_IOC_MARKER, &arg) 8<---- If so, maybe it would simpler just to go for classic size/data structure? 8<----- struct perf_event_marker { uint32_t size; void *data; } 8<----- This would directly map into struct perf_raw_record... Paweł -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 09/12/2014 08:57 AM, Pawel Moll wrote: > On Fri, 2014-09-12 at 13:43 +0100, Christopher Covington wrote: >> Just to ask the dumb questions in case the answers I've come up with are >> wrong: What is PAGE_SIZE on an arm64 kernel? > > It's either 4 or 64k, depending on CONFIG_ARM64_64K_PAGES. > >> How does userspace know? >> > #include <unistd.h> > #include <stdio.h> > > int main(void) > { > printf("%ld\n", sysconf(_SC_PAGESIZE)); > return 0; > } Oh excellent, that actually works. Based on a misreading of the glibc code I thought it was hard-coded to 64K. Thanks, Christopher
Em Fri, Sep 12, 2014 at 02:58:55PM +0100, Pawel Moll escreveu: > On Fri, 2014-09-12 at 14:49 +0100, Arnaldo Carvalho de Melo wrote: > > Perhaps both? I.e. an u64 followed from a string, if the u64 is zero, > > then there is a string right after it? > How would this look like in userspace? Something like this? > 8<---- > struct perf_event_marker { > uint64_t value; > char *string; > } arg; > arg.value = 0x1234; > /* or */ > arg.value = 0; > arg.string = "abcd"; > ioctl(fd, PERF_EVENT_IOC_MARKER, &arg) > 8<---- > If so, maybe it would simpler just to go for classic size/data > structure? > 8<----- > struct perf_event_marker { > uint32_t size; > void *data; > } > 8<----- > This would directly map into struct perf_raw_record... I can see the usefulness of having it all, i.e. if we do just: perf trace --pid `pidof some-tool-in-debug-mode-using-this-interface` Then 'perf trace' doesn't know about any binary format a tool may have, getting strings there (hey, LD_PRELOADing some logging library to hook into this comes to mind) and having it merged with other events (syscalls, pagefaults, etc) looks useful. As well as some specialized version of 'perf trace' that knows about some binary protocol that would get app specific stats or lock status, etc, perhaps even plugins for 'perf trace' that would be selected by that first u64? Also seems useful. I.e. having a way to provide just strings and another that would allow passing perf_raw_record. - Arnaldo - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 9/12/14, 4:48 AM, Pawel Moll wrote: > This patch adds a PERF_COUNT_SW_MARKER event type, which > can be requested by user and a PERF_EVENT_IOC_MARKER > ioctl command which will inject an event of said type into > the perf buffer. The ioctl can take a zero-terminated > string argument, similar to tracing_marker in ftrace, > which will be kept in the "raw" field of the sample. > > The main use case for this is synchronisation of > performance data generated in user space with the perf > stream coming from the kernel. For example, the marker > can be inserted by a JIT engine after it generated > portion of the code, but before the code is executed > for the first time, allowing the post-processor to > pick the correct debugging information. Other example > is a system profiling tool taking data from other > sources than just perf, which generates a marker > at the beginning at at the end of the session > (also possibly periodically during the session) to > synchronise kernel timestamps with clock values > obtained in userspace (gtod or raw_monotonic). Seems really similar to what I proposed in the past: https://lkml.org/lkml/2011/2/27/159 Which was rejected. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Em Fri, Sep 12, 2014 at 10:37:39AM -0700, David Ahern escreveu: > On 9/12/14, 4:48 AM, Pawel Moll wrote: > >This patch adds a PERF_COUNT_SW_MARKER event type, which > >can be requested by user and a PERF_EVENT_IOC_MARKER > >ioctl command which will inject an event of said type into > >the perf buffer. The ioctl can take a zero-terminated > >string argument, similar to tracing_marker in ftrace, > >which will be kept in the "raw" field of the sample. > > > >The main use case for this is synchronisation of > >performance data generated in user space with the perf > >stream coming from the kernel. For example, the marker > >can be inserted by a JIT engine after it generated > >portion of the code, but before the code is executed > >for the first time, allowing the post-processor to > >pick the correct debugging information. Other example > >is a system profiling tool taking data from other > >sources than just perf, which generates a marker > >at the beginning at at the end of the session > >(also possibly periodically during the session) to > >synchronise kernel timestamps with clock values > >obtained in userspace (gtod or raw_monotonic). > > Seems really similar to what I proposed in the past: > > https://lkml.org/lkml/2011/2/27/159 > > Which was rejected. I took a look at that thread, but just barely, emphasis on that. Injecting something from userspace, a la ftrace, seems to be something, as tglx mentioned, "buried" in that patchset. - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 9/12/14, 2:44 PM, Arnaldo Carvalho de Melo wrote: > Em Fri, Sep 12, 2014 at 10:37:39AM -0700, David Ahern escreveu: >> On 9/12/14, 4:48 AM, Pawel Moll wrote: >>> This patch adds a PERF_COUNT_SW_MARKER event type, which >>> can be requested by user and a PERF_EVENT_IOC_MARKER >>> ioctl command which will inject an event of said type into >>> the perf buffer. The ioctl can take a zero-terminated >>> string argument, similar to tracing_marker in ftrace, >>> which will be kept in the "raw" field of the sample. >>> >>> The main use case for this is synchronisation of >>> performance data generated in user space with the perf >>> stream coming from the kernel. For example, the marker >>> can be inserted by a JIT engine after it generated >>> portion of the code, but before the code is executed >>> for the first time, allowing the post-processor to >>> pick the correct debugging information. Other example >>> is a system profiling tool taking data from other >>> sources than just perf, which generates a marker >>> at the beginning at at the end of the session >>> (also possibly periodically during the session) to >>> synchronise kernel timestamps with clock values >>> obtained in userspace (gtod or raw_monotonic). >> >> Seems really similar to what I proposed in the past: >> >> https://lkml.org/lkml/2011/2/27/159 >> >> Which was rejected. > > I took a look at that thread, but just barely, emphasis on that. > > Injecting something from userspace, a la ftrace, seems to be something, > as tglx mentioned, "buried" in that patchset. Thomas object to an ioctl buried deep in a patch -- newbie mistake. Peter objected to the ioctl https://lkml.org/lkml/2011/3/1/229 It was not userspace injecting random data into the stream but rather forcing the sample to be generated and added to the stream. David David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Sun, 2014-09-14 at 16:43 +0100, David Ahern wrote: > >> Seems really similar to what I proposed in the past: Yeah, it wasn't really hard to come with similar conclusions :-) > >> https://lkml.org/lkml/2011/2/27/159 > >> > >> Which was rejected. > > > > I took a look at that thread, but just barely, emphasis on that. > > > > Injecting something from userspace, a la ftrace, seems to be something, > > as tglx mentioned, "buried" in that patchset. > > Thomas object to an ioctl buried deep in a patch -- newbie mistake. > > Peter objected to the ioctl https://lkml.org/lkml/2011/3/1/229 > > It was not userspace injecting random data into the stream but rather > forcing the sample to be generated and added to the stream. I would like to hear from Peter and others. If not here, I'll get them to talk next month on Linux Plumbers :-) Pawel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Fri, 2014-09-12 at 17:19 +0100, Arnaldo Carvalho de Melo wrote: > Em Fri, Sep 12, 2014 at 02:58:55PM +0100, Pawel Moll escreveu: > > On Fri, 2014-09-12 at 14:49 +0100, Arnaldo Carvalho de Melo wrote: > > > Perhaps both? I.e. an u64 followed from a string, if the u64 is zero, > > > then there is a string right after it? > > > How would this look like in userspace? Something like this? > > > 8<---- > > struct perf_event_marker { > > uint64_t value; > > char *string; > > } arg; > > > arg.value = 0x1234; > > > /* or */ > > > arg.value = 0; > > arg.string = "abcd"; > > > ioctl(fd, PERF_EVENT_IOC_MARKER, &arg) > > 8<---- > > > If so, maybe it would simpler just to go for classic size/data > > structure? > > > 8<----- > > struct perf_event_marker { > > uint32_t size; > > void *data; > > } > > 8<----- > > > This would directly map into struct perf_raw_record... > > I can see the usefulness of having it all, i.e. if we do just: > > perf trace --pid `pidof some-tool-in-debug-mode-using-this-interface` Hm. I haven't thought about a situation when 3rd party wants to inject something into "my" data stream... I guess it could be implemented (a "pid" member of the struct perf_event_marker with default 0 meaning "myself"?), but will definitely complicate the patch. Should I have a look at it now or maybe leave it till we get a general agreement about the marker ioctl existence? > Then 'perf trace' doesn't know about any binary format a tool may have, > getting strings there (hey, LD_PRELOADing some logging library to hook > into this comes to mind) and having it merged with other events > (syscalls, pagefaults, etc) looks useful. But do you still mean a "magic" u64 before the rest? Injecting a string would just mean: marker.size = strlen(s) + 1; marker.data = s; > As well as some specialized version of 'perf trace' that knows about > some binary protocol that would get app specific stats or lock status, > etc, perhaps even plugins for 'perf trace' that would be selected by > that first u64? Also seems useful. > > I.e. having a way to provide just strings and another that would allow > passing perf_raw_record. Sounds interesting. But then maybe this stuff shouldn't go into "raw" then? It could be something like this in the sample: { u64 type; /* 0 means zero-terminated string in data */ u32 size; char data[size]; } && PERF_SAMPLE_MARKER Pawel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Em Mon, Sep 15, 2014 at 06:27:14PM +0100, Pawel Moll escreveu: > On Fri, 2014-09-12 at 17:19 +0100, Arnaldo Carvalho de Melo wrote: > > Em Fri, Sep 12, 2014 at 02:58:55PM +0100, Pawel Moll escreveu: > > > On Fri, 2014-09-12 at 14:49 +0100, Arnaldo Carvalho de Melo wrote: > > > > Perhaps both? I.e. an u64 followed from a string, if the u64 is zero, > > > > then there is a string right after it? > > > > > How would this look like in userspace? Something like this? > > > > > 8<---- > > > struct perf_event_marker { > > > uint64_t value; > > > char *string; > > > } arg; > > > > > arg.value = 0x1234; > > > > > /* or */ > > > > > arg.value = 0; > > > arg.string = "abcd"; > > > > > ioctl(fd, PERF_EVENT_IOC_MARKER, &arg) > > > 8<---- > > > > > If so, maybe it would simpler just to go for classic size/data > > > structure? > > > > > 8<----- > > > struct perf_event_marker { > > > uint32_t size; > > > void *data; > > > } > > > 8<----- > > > > > This would directly map into struct perf_raw_record... > > > > I can see the usefulness of having it all, i.e. if we do just: > > > > perf trace --pid `pidof some-tool-in-debug-mode-using-this-interface` > > Hm. I haven't thought about a situation when 3rd party wants to inject > something into "my" data stream... I guess it could be implemented (a I was thinking about intercepting calls that pass some logging data, as strings, and 'tee' them to the 'perf trace' 'data stream'. > "pid" member of the struct perf_event_marker with default 0 meaning Humm, Isn't PERF_SAMPLE_TID enough for that? > "myself"?), but will definitely complicate the patch. Should I have a > look at it now or maybe leave it till we get a general agreement about > the marker ioctl existence? > > > Then 'perf trace' doesn't know about any binary format a tool may have, > > getting strings there (hey, LD_PRELOADing some logging library to hook > > into this comes to mind) and having it merged with other events > > (syscalls, pagefaults, etc) looks useful. > > But do you still mean a "magic" u64 before the rest? Injecting a string > would just mean: > > marker.size = strlen(s) + 1; > marker.data = s; > > > As well as some specialized version of 'perf trace' that knows about > > some binary protocol that would get app specific stats or lock status, > > etc, perhaps even plugins for 'perf trace' that would be selected by > > that first u64? Also seems useful. > > > > I.e. having a way to provide just strings and another that would allow > > passing perf_raw_record. > > Sounds interesting. But then maybe this stuff shouldn't go into "raw" > then? It could be something like this in the sample: > > { u64 type; /* 0 means zero-terminated string in data */ > u32 size; > char data[size]; } && PERF_SAMPLE_MARKER Yes, this is how I think it should be. > Pawel > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
* David Ahern <dsahern@gmail.com> wrote: > On 9/12/14, 2:44 PM, Arnaldo Carvalho de Melo wrote: > >Em Fri, Sep 12, 2014 at 10:37:39AM -0700, David Ahern escreveu: > >>On 9/12/14, 4:48 AM, Pawel Moll wrote: > >>>This patch adds a PERF_COUNT_SW_MARKER event type, which > >>>can be requested by user and a PERF_EVENT_IOC_MARKER > >>>ioctl command which will inject an event of said type into > >>>the perf buffer. The ioctl can take a zero-terminated > >>>string argument, similar to tracing_marker in ftrace, > >>>which will be kept in the "raw" field of the sample. > >>> > >>>The main use case for this is synchronisation of > >>>performance data generated in user space with the perf > >>>stream coming from the kernel. For example, the marker > >>>can be inserted by a JIT engine after it generated > >>>portion of the code, but before the code is executed > >>>for the first time, allowing the post-processor to > >>>pick the correct debugging information. Other example > >>>is a system profiling tool taking data from other > >>>sources than just perf, which generates a marker > >>>at the beginning at at the end of the session > >>>(also possibly periodically during the session) to > >>>synchronise kernel timestamps with clock values > >>>obtained in userspace (gtod or raw_monotonic). > >> > >>Seems really similar to what I proposed in the past: > >> > >>https://lkml.org/lkml/2011/2/27/159 > >> > >>Which was rejected. > > > >I took a look at that thread, but just barely, emphasis on that. > > > >Injecting something from userspace, a la ftrace, seems to be something, > >as tglx mentioned, "buried" in that patchset. > > Thomas object to an ioctl buried deep in a patch -- newbie > mistake. > > Peter objected to the ioctl https://lkml.org/lkml/2011/3/1/229 > > It was not userspace injecting random data into the stream but > rather forcing the sample to be generated and added to the > stream. I think adding an ioctl to inject user-provided data into the event stream is sensible, as long as there's a separate 'user generated data' event for it, etc. The main usecase I could see would be to introduce a perf_printf() variant, supported by 'perf trace' by default, to add various tracable printouts to apps. Timestamps generated by apps would be another usecase. It would probably be wise to add a 32-bit (or 64-bit) message type ID, plus a length field, with a message type registry somewhere in tools/perf/ (and reference implementation for each new subtype), to keep things organized yet flexible going forward. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Mon, 2014-09-15 at 19:31 +0100, Arnaldo Carvalho de Melo wrote: > Em Mon, Sep 15, 2014 at 06:27:14PM +0100, Pawel Moll escreveu: > > On Fri, 2014-09-12 at 17:19 +0100, Arnaldo Carvalho de Melo wrote: > > > Em Fri, Sep 12, 2014 at 02:58:55PM +0100, Pawel Moll escreveu: > > > > On Fri, 2014-09-12 at 14:49 +0100, Arnaldo Carvalho de Melo wrote: > > > > > Perhaps both? I.e. an u64 followed from a string, if the u64 is zero, > > > > > then there is a string right after it? > > > > > > > How would this look like in userspace? Something like this? > > > > > > > 8<---- > > > > struct perf_event_marker { > > > > uint64_t value; > > > > char *string; > > > > } arg; > > > > > > > arg.value = 0x1234; > > > > > > > /* or */ > > > > > > > arg.value = 0; > > > > arg.string = "abcd"; > > > > > > > ioctl(fd, PERF_EVENT_IOC_MARKER, &arg) > > > > 8<---- > > > > > > > If so, maybe it would simpler just to go for classic size/data > > > > structure? > > > > > > > 8<----- > > > > struct perf_event_marker { > > > > uint32_t size; > > > > void *data; > > > > } > > > > 8<----- > > > > > > > This would directly map into struct perf_raw_record... > > > > > > I can see the usefulness of having it all, i.e. if we do just: > > > > > > perf trace --pid `pidof some-tool-in-debug-mode-using-this-interface` > > > > Hm. I haven't thought about a situation when 3rd party wants to inject > > something into "my" data stream... I guess it could be implemented (a > > I was thinking about intercepting calls that pass some logging data, as > strings, and 'tee' them to the 'perf trace' 'data stream'. Right, ok, like LD_PRELOADing printf (a stupid example of course) and piping it inside perf... So if I'm getting it right, it's the perf process that would eventually do the ioctl(PERF_EVENT_IOC_MARKER), not the traced process, correct? This makes sense. Another use case for ioctl justification, thanks :-) > > "myself"?), but will definitely complicate the patch. Should I have a > > look at it now or maybe leave it till we get a general agreement about > > the marker ioctl existence? > > > > > Then 'perf trace' doesn't know about any binary format a tool may have, > > > getting strings there (hey, LD_PRELOADing some logging library to hook > > > into this comes to mind) and having it merged with other events > > > (syscalls, pagefaults, etc) looks useful. > > > > But do you still mean a "magic" u64 before the rest? Injecting a string > > would just mean: > > > > marker.size = strlen(s) + 1; > > marker.data = s; > > > > > As well as some specialized version of 'perf trace' that knows about > > > some binary protocol that would get app specific stats or lock status, > > > etc, perhaps even plugins for 'perf trace' that would be selected by > > > that first u64? Also seems useful. > > > > > > I.e. having a way to provide just strings and another that would allow > > > passing perf_raw_record. > > > > Sounds interesting. But then maybe this stuff shouldn't go into "raw" > > then? It could be something like this in the sample: > > > > { u64 type; /* 0 means zero-terminated string in data */ > > u32 size; > > char data[size]; } && PERF_SAMPLE_MARKER > > Yes, this is how I think it should be. Seems that Ingo had exactly the same thing on mind. I'll get a patch done. Paweł -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Tue, 2014-09-16 at 08:44 +0100, Ingo Molnar wrote: > I think adding an ioctl to inject user-provided data into the > event stream is sensible, as long as there's a separate 'user > generated data' event for it, etc. > > The main usecase I could see would be to introduce a > perf_printf() variant, supported by 'perf trace' by default, to > add various tracable printouts to apps. > > Timestamps generated by apps would be another usecase. It would > probably be wise to add a 32-bit (or 64-bit) message type ID, > plus a length field, with a message type registry somewhere in > tools/perf/ (and reference implementation for each new subtype), > to keep things organized yet flexible going forward. Right, so this is pretty much what I got talking to Arnaldo... > { u64 type; /* 0 means zero-terminated string in data */ > u32 size; > char data[size]; } && PERF_SAMPLE_MARKER ... with one type - 0 - defined as a "universal" string (so any possible tool knows what to do about it), the rest being left to userspace (this "registry" you mention). Before I proceed any further, is the term "marker" acceptable? Maybe a "printf" instead? Or a "log"? As we know naming is often single most discussed subject when it comes to new things in the kernel ;-) Pawel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
* Pawel Moll <pawel.moll@arm.com> wrote: > On Tue, 2014-09-16 at 08:44 +0100, Ingo Molnar wrote: > > I think adding an ioctl to inject user-provided data into the > > event stream is sensible, as long as there's a separate 'user > > generated data' event for it, etc. > > > > The main usecase I could see would be to introduce a > > perf_printf() variant, supported by 'perf trace' by default, to > > add various tracable printouts to apps. > > > > Timestamps generated by apps would be another usecase. It would > > probably be wise to add a 32-bit (or 64-bit) message type ID, > > plus a length field, with a message type registry somewhere in > > tools/perf/ (and reference implementation for each new subtype), > > to keep things organized yet flexible going forward. > > Right, so this is pretty much what I got talking to Arnaldo... > > > { u64 type; /* 0 means zero-terminated string in data */ > > u32 size; > > char data[size]; } && PERF_SAMPLE_MARKER > > ... with one type - 0 - defined as a "universal" string (so any > possible tool knows what to do about it), the rest being left > to userspace (this "registry" you mention). > > Before I proceed any further, is the term "marker" acceptable? > Maybe a "printf" instead? Or a "log"? As we know naming is > often single most discussed subject when it comes to new things > in the kernel ;-) Well, it's a user-space generated trace/event entry, so lets call it that? Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h index e5a75c5..83b0f5b 100644 --- a/include/uapi/linux/perf_event.h +++ b/include/uapi/linux/perf_event.h @@ -110,6 +110,7 @@ enum perf_sw_ids { PERF_COUNT_SW_ALIGNMENT_FAULTS = 7, PERF_COUNT_SW_EMULATION_FAULTS = 8, PERF_COUNT_SW_DUMMY = 9, + PERF_COUNT_SW_MARKER = 10, PERF_COUNT_SW_MAX, /* non-ABI */ }; @@ -350,6 +351,7 @@ 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_MARKER _IOR('$', 8, char *) enum perf_event_ioc_flags { PERF_IOC_FLAG_GROUP = 1U << 0, diff --git a/kernel/events/core.c b/kernel/events/core.c index df093e3..dbce284 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -3655,6 +3655,7 @@ static inline int perf_fget_light(int fd, struct fd *p) 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_sw_event_marker(struct perf_event *event, char __user *arg); static long perf_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3709,6 +3710,9 @@ 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_MARKER: + return perf_sw_event_marker(event, (char __user *)arg); + default: return -ENOTTY; } @@ -3728,6 +3732,7 @@ static long perf_compat_ioctl(struct file *file, unsigned int cmd, switch (_IOC_NR(cmd)) { case _IOC_NR(PERF_EVENT_IOC_SET_FILTER): case _IOC_NR(PERF_EVENT_IOC_ID): + case _IOC_NR(PERF_EVENT_IOC_MARKER): /* Fix up pointer size (usually 4 -> 8 in 32-on-64-bit case */ if (_IOC_SIZE(cmd) == sizeof(compat_uptr_t)) { cmd &= ~IOCSIZE_MASK; @@ -5960,6 +5965,44 @@ static struct pmu perf_swevent = { .event_idx = perf_swevent_event_idx, }; +static int perf_sw_event_marker(struct perf_event *event, char __user *arg) +{ + struct perf_sample_data data; + struct pt_regs *regs = current_pt_regs(); + struct perf_raw_record raw = { 0, }; + + if (!static_key_false(&perf_swevent_enabled[PERF_COUNT_SW_MARKER])) + return 0; + + perf_sample_data_init(&data, 0, 0); + + if (arg) { + long len = strnlen_user(arg, PAGE_SIZE); + + if (len) { + raw.size = ALIGN(len + sizeof(u32), sizeof(u64)) + - sizeof(u32); + raw.data = kzalloc(raw.size, GFP_KERNEL); + if (!raw.data) + return -ENOMEM; + + if (copy_from_user(raw.data, arg, len)) { + kfree(raw.data); + return -EFAULT; + } + + data.raw = &raw; + } + } + + perf_event_output(event, &data, regs); + + if (raw.size) + kfree(raw.data); + + return 0; +} + #ifdef CONFIG_EVENT_TRACING static int perf_tp_filter_match(struct perf_event *event,
This patch adds a PERF_COUNT_SW_MARKER event type, which can be requested by user and a PERF_EVENT_IOC_MARKER ioctl command which will inject an event of said type into the perf buffer. The ioctl can take a zero-terminated string argument, similar to tracing_marker in ftrace, which will be kept in the "raw" field of the sample. The main use case for this is synchronisation of performance data generated in user space with the perf stream coming from the kernel. For example, the marker can be inserted by a JIT engine after it generated portion of the code, but before the code is executed for the first time, allowing the post-processor to pick the correct debugging information. Other example is a system profiling tool taking data from other sources than just perf, which generates a marker at the beginning at at the end of the session (also possibly periodically during the session) to synchronise kernel timestamps with clock values obtained in userspace (gtod or raw_monotonic). Signed-off-by: Pawel Moll <pawel.moll@arm.com> --- include/uapi/linux/perf_event.h | 2 ++ kernel/events/core.c | 43 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+)