diff mbox

[V3,1/6] perf/core: Adding PMU driver specific configuration

Message ID 1469742143-22245-2-git-send-email-mathieu.poirier@linaro.org
State New
Headers show

Commit Message

Mathieu Poirier July 28, 2016, 9:42 p.m. UTC 
This patch somewhat mimics the work done on address filters to
add the infrastructure needed to pass PMU specific HW
configuration to the driver before a session starts.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

---
 include/linux/perf_event.h            |  9 +++++++++
 include/uapi/linux/perf_event.h       |  1 +
 kernel/events/core.c                  | 16 ++++++++++++++++
 tools/include/uapi/linux/perf_event.h |  1 +
 4 files changed, 27 insertions(+)

-- 
2.7.4

Comments

Mathieu Poirier Aug. 5, 2016, 3:35 p.m. UTC | #1 
On 4 August 2016 at 10:58, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Jul 28, 2016 at 03:42:18PM -0600, Mathieu Poirier wrote:

>> This patch somewhat mimics the work done on address filters to

>> add the infrastructure needed to pass PMU specific HW

>> configuration to the driver before a session starts.

>>

>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>

>> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h

>> index c66a485a24ac..90fbc5fd3925 100644

>> --- a/include/uapi/linux/perf_event.h

>> +++ b/include/uapi/linux/perf_event.h

>> @@ -407,6 +407,7 @@ struct perf_event_attr {

>>  #define PERF_EVENT_IOC_ID            _IOR('$', 7, __u64 *)

>>  #define PERF_EVENT_IOC_SET_BPF               _IOW('$', 8, __u32)

>>  #define PERF_EVENT_IOC_PAUSE_OUTPUT  _IOW('$', 9, __u32)

>> +#define PERF_EVENT_IOC_SET_DRV_CONFIGS       _IOW('$', 10, char *)

>

> Please also do a manpages patch.


Patch 3/6 in this set documents the new option
(tools/perf/Documentation/perf-record.tx).  Is this what you were
looking for?  If not please expand on the information you want to see
added add and where.

Thanks,
Mathieu
Mathieu Poirier Aug. 5, 2016, 3:41 p.m. UTC | #2 
On 4 August 2016 at 10:59, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Jul 28, 2016 at 03:42:18PM -0600, Mathieu Poirier wrote:

>> This patch somewhat mimics the work done on address filters to

>> add the infrastructure needed to pass PMU specific HW

>> configuration to the driver before a session starts.

>

> I'm thinking we want to specify a syntax and validate the string matches

> the syntax in the generic code.


The syntax is checked in the lexer making sure that nothing other than
@cfg or @cfg=config is sent to the kernel.  From there validation is
done in the PMU driver that implements the set_drv_configs()
interface.

I am not sure to get you point here - can I ask you to provide more details?

Thanks,
Mathieu
Mathieu Poirier Aug. 10, 2016, 11:07 p.m. UTC | #3 
On 5 August 2016 at 09:53, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Aug 05, 2016 at 09:35:05AM -0600, Mathieu Poirier wrote:

>> On 4 August 2016 at 10:58, Peter Zijlstra <peterz@infradead.org> wrote:

>> > On Thu, Jul 28, 2016 at 03:42:18PM -0600, Mathieu Poirier wrote:

>> >> This patch somewhat mimics the work done on address filters to

>> >> add the infrastructure needed to pass PMU specific HW

>> >> configuration to the driver before a session starts.

>> >>

>> >> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>> >

>> >> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h

>> >> index c66a485a24ac..90fbc5fd3925 100644

>> >> --- a/include/uapi/linux/perf_event.h

>> >> +++ b/include/uapi/linux/perf_event.h

>> >> @@ -407,6 +407,7 @@ struct perf_event_attr {

>> >>  #define PERF_EVENT_IOC_ID            _IOR('$', 7, __u64 *)

>> >>  #define PERF_EVENT_IOC_SET_BPF               _IOW('$', 8, __u32)

>> >>  #define PERF_EVENT_IOC_PAUSE_OUTPUT  _IOW('$', 9, __u32)

>> >> +#define PERF_EVENT_IOC_SET_DRV_CONFIGS       _IOW('$', 10, char *)

>> >

>> > Please also do a manpages patch.

>>

>> Patch 3/6 in this set documents the new option

>> (tools/perf/Documentation/perf-record.tx).  Is this what you were

>> looking for?  If not please expand on the information you want to see

>> added add and where.

>

> Since you add an IOCTL (with preferably more structure than present in

> this patch, see the other email) this needs to be documented in the

> syscall manpage.

>

>   http://git.kernel.org/cgit/docs/man-pages/man-pages.git/tree/man2/perf_event_open.2

>

>   http://www.man7.org/linux/man-pages/man2/perf_event_open.2.html


After a little bit of digging around I understand that manpages have
to be written _after_ the new ioctl call has been added - at least
that's what I deduce when looking at what Vince Weaver did for the BPF
support:

commit b0f7b411bed0505937f0f51d6499d0c6c56f4b8c
Author: Vince Weaver <vincent.weaver@maine.edu>
Date:   Thu Jul 23 13:10:21 2015 -0400

    perf_event_open.2: 4.1 PERF_EVENT_IOC_SET_BPF support

    This manpage patch relates to the addition of the
    PERF_EVENT_IOC_SET_BPF ioctl in the following commit:

        commit 2541517c32be2531e0da59dfd7efc1ce844644f5
        Author: Alexei Starovoitov <ast@plumgrid.com>

        tracing, perf: Implement BPF programs attached to kprobes

        Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>

        Reviewed-by: Steven Rostedt <rostedt@goodmis.org>

        Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

        Cc: Andrew Morton <akpm@linux-foundation.org>
        Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
        Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
        Cc: Daniel Borkmann <daniel@iogearbox.net>
        Cc: David S. Miller <davem@davemloft.net>
        Cc: Jiri Olsa <jolsa@redhat.com>
        Cc: Linus Torvalds <torvalds@linux-foundation.org>
        Cc: Namhyung Kim <namhyung@kernel.org>
        Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
        Cc: Peter Zijlstra <peterz@infradead.org>
        Link: http://lkml.kernel.org/r/1427312966-8434-4-git-send-email-ast@plumgrid.com
        Signed-off-by: Ingo Molnar <mingo@kernel.org>


    Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>

    Signed-off-by: Michael Kerrisk <mtk.manpages@gmail.com>


Am I correct here or you want to proceed differently?

Thanks for the guidance,
Mathieu
diff mbox

Patch

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 7921f4f20a58..59d61a12cf9d 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -168,6 +168,9 @@  struct hw_perf_event {
 	/* Last sync'ed generation of filters */
 	unsigned long			addr_filters_gen;
 
+	/* HW specific configuration */
+	void				*drv_configs;
+
 /*
  * hw_perf_event::state flags; used to track the PERF_EF_* state.
  */
@@ -442,6 +445,12 @@  struct pmu {
 	 * Filter events for PMU-specific reasons.
 	 */
 	int (*filter_match)		(struct perf_event *event); /* optional */
+
+	/*
+	 * PMU driver specific configuration.
+	 */
+	int (*set_drv_configs)		(struct perf_event *event,
+					 void __user *arg); /* optional */
 };
 
 /**
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index c66a485a24ac..90fbc5fd3925 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -407,6 +407,7 @@  struct perf_event_attr {
 #define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
 #define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
 #define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_SET_DRV_CONFIGS	_IOW('$', 10, 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 79dae188a987..9208e6ec036f 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4457,6 +4457,8 @@  static int perf_event_set_output(struct perf_event *event,
 				 struct perf_event *output_event);
 static int perf_event_set_filter(struct perf_event *event, void __user *arg);
 static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd);
+static int perf_event_set_drv_configs(struct perf_event *event,
+				      void __user *arg);
 
 static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned long arg)
 {
@@ -4526,6 +4528,10 @@  static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
 		rcu_read_unlock();
 		return 0;
 	}
+
+	case PERF_EVENT_IOC_SET_DRV_CONFIGS:
+		return perf_event_set_drv_configs(event, (void __user *)arg);
+
 	default:
 		return -ENOTTY;
 	}
@@ -4558,6 +4564,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_SET_DRV_CONFIGS):
 		/* Fix up pointer size (usually 4 -> 8 in 32-on-64-bit case */
 		if (_IOC_SIZE(cmd) == sizeof(compat_uptr_t)) {
 			cmd &= ~IOCSIZE_MASK;
@@ -7633,6 +7640,15 @@  void perf_bp_event(struct perf_event *bp, void *data)
 }
 #endif
 
+static int perf_event_set_drv_configs(struct perf_event *event,
+				  void __user *arg)
+{
+	if (!event->pmu->set_drv_configs)
+		return -EINVAL;
+
+	return event->pmu->set_drv_configs(event, arg);
+}
+
 /*
  * Allocate a new address filter
  */
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index c66a485a24ac..90fbc5fd3925 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -407,6 +407,7 @@  struct perf_event_attr {
 #define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
 #define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
 #define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_SET_DRV_CONFIGS	_IOW('$', 10, char *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,