linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/2] Notifications for perf sideband events
@ 2017-06-13 11:33 Naveen N. Rao
  2017-06-13 11:33 ` [RFC PATCH v2 1/2] kernel/events: Add option to notify through signals on wakeup Naveen N. Rao
  2017-06-13 11:33 ` [RFC PATCH v2 2/2] kernel/events: Add option to enable counting sideband events in wakeup_events Naveen N. Rao
  0 siblings, 2 replies; 5+ messages in thread
From: Naveen N. Rao @ 2017-06-13 11:33 UTC (permalink / raw)
  To: Peter Zijlstra, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar
  Cc: linux-kernel

v2:
Here is a slightly different approach compared to v1: instead of
introducing a new IOCTL, we ride on top of the existing support for
ring buffer wakeup to generate signals as desired. Counting sideband
events still requires some changes in the output path, but in normal
cases, it ends up being just a comparison.

The test program below needs to be changed for this version. Add below
and remove the ioctl call for PERF_EVENT_IOC_COUNT_RECORDS:
	pe.signal_on_wakeup = 1;
	pe.count_sb_events = 1;
	pe.wakeup_events = 2;

Also, I have not handled PERF_EVENT_IOC_REFRESH in this version, so
IOC_ENABLE needs to be used.

v1:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1414197.html

Cover letter from v1:
--------------------

Currently, there is no way to ask for signals to be delivered when a
certain number of sideband events have been logged into the ring
buffer. This is problematic if we are only interested in, say, context
switch events. This patch provides for a way to achieve this.

As noted, this is a RFC and I am not too specific about the interface or
the ioctl name. Kindly suggest if you think there is a better way to
achieve this.

- Naveen

---
Here is a sample program demonstrating the same:

    #define _GNU_SOURCE

    #include <stdlib.h>
    #include <stdio.h>
    #include <unistd.h>
    #include <fcntl.h>
    #include <string.h>
    #include <signal.h>
    #include <sys/ioctl.h>
    #include <sys/mman.h>
    #include <linux/perf_event.h>
    #include <asm/unistd.h>

    static long
    perf_event_open(struct perf_event_attr *hw_event, pid_t pid,
		   int cpu, int group_fd, unsigned long flags)
    {
	return syscall(__NR_perf_event_open, hw_event, pid, cpu,
		      group_fd, flags);
    }

    static void sigio_handler(int n, siginfo_t *info, void *uc)
    {
	fprintf (stderr, "Caught %s\n", info->si_code == POLL_HUP ? "POLL_HUP" :
		       (info->si_code == POLL_IN ? "POLL_IN" : "other signal"));

	if (ioctl(info->si_fd, PERF_EVENT_IOC_REFRESH, 2) == -1)
	    perror("SIGIO: IOC_REFRESH");
    }

    int main(int argc, char **argv)
    {
	struct perf_event_attr pe;
	struct sigaction act;
	int fd;
	void *buf;

	memset(&act, 0, sizeof(act));
	act.sa_sigaction = sigio_handler;
	act.sa_flags = SA_SIGINFO;
	sigaction(SIGIO, &act, 0);

	memset(&pe, 0, sizeof(struct perf_event_attr));
	pe.size = sizeof(struct perf_event_attr);
	pe.type = PERF_TYPE_SOFTWARE;
	pe.config = PERF_COUNT_SW_DUMMY;
	pe.disabled = 1;
	pe.sample_period = 1;
	pe.context_switch = 1;

	fd = perf_event_open(&pe, 0, -1, -1, 0);
	if (fd == -1) {
	    fprintf(stderr, "Error opening leader %lx\n", pe.config);
	    exit(EXIT_FAILURE);
	}

	buf = mmap(NULL, sysconf(_SC_PAGESIZE) * 2, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
	if (buf == MAP_FAILED) {
	    fprintf(stderr, "Can't mmap buffer\n");
	    return -1;
	}

	if (fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_ASYNC) == -1)
	    return -2;

	if (fcntl(fd, F_SETSIG, SIGIO) == -1)
	    return -3;

	if (fcntl(fd, F_SETOWN, getpid()) == -1)
	    return -4;

	if (ioctl(fd, PERF_EVENT_IOC_COUNT_RECORDS, 0) == -1)
	    return -5;

	if (ioctl(fd, PERF_EVENT_IOC_REFRESH, 2) == -1)
	    return -6;

	fprintf (stderr, "Sleep 1\n");
	sleep(1);
	fprintf (stderr, "Sleep 2\n");
	sleep(1);
	fprintf (stderr, "Sleep 3\n");
	sleep(1);

	/* Disable the event counter */
	ioctl(fd, PERF_EVENT_IOC_DISABLE, 1);

	close(fd);

	return 0;
    }

A sample output:
    $ time ./cs
    Sleep 1
    Caught POLL_HUP
    Sleep 2
    Caught POLL_HUP
    Sleep 3
    Caught POLL_HUP

    real	0m3.060s
    user	0m0.001s
    sys	0m0.005s


Naveen N. Rao (2):
  kernel/events: Add option to notify through signals on wakeup
  kernel/events: Add option to enable counting sideband events in
    wakeup_events

 include/uapi/linux/perf_event.h |  4 +++-
 kernel/events/core.c            | 14 ++++++++------
 kernel/events/ring_buffer.c     | 16 ++++++++++++++++
 3 files changed, 27 insertions(+), 7 deletions(-)

-- 
2.12.2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RFC PATCH v2 1/2] kernel/events: Add option to notify through signals on wakeup
  2017-06-13 11:33 [RFC PATCH v2 0/2] Notifications for perf sideband events Naveen N. Rao
@ 2017-06-13 11:33 ` Naveen N. Rao
  2017-06-13 14:45   ` Jiri Olsa
  2017-06-13 11:33 ` [RFC PATCH v2 2/2] kernel/events: Add option to enable counting sideband events in wakeup_events Naveen N. Rao
  1 sibling, 1 reply; 5+ messages in thread
From: Naveen N. Rao @ 2017-06-13 11:33 UTC (permalink / raw)
  To: Peter Zijlstra, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar
  Cc: linux-kernel

Add a new option 'signal_on_wakeup' to request for a signal to be
delivered on ring buffer wakeup controlled through watermark and
{wakeup_events, wakeup_watermark}.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 include/uapi/linux/perf_event.h |  3 ++-
 kernel/events/core.c            | 12 +++++++-----
 kernel/events/ring_buffer.c     |  3 +++
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index b1c0b187acfe..e5810b1d74a4 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -345,7 +345,8 @@ struct perf_event_attr {
 				context_switch :  1, /* context switch data */
 				write_backward :  1, /* Write ring buffer from end to beginning */
 				namespaces     :  1, /* include namespaces data */
-				__reserved_1   : 35;
+				signal_on_wakeup : 1, /* send signal on wakeup */
+				__reserved_1   : 34;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 6c4e523dc1e2..73ad30e124e5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7362,12 +7362,14 @@ static int __perf_event_overflow(struct perf_event *event,
 	 * events
 	 */
 
-	event->pending_kill = POLL_IN;
-	if (events && atomic_dec_and_test(&event->event_limit)) {
-		ret = 1;
-		event->pending_kill = POLL_HUP;
+	if (!event->attr.signal_on_wakeup) {
+		event->pending_kill = POLL_IN;
+		if (events && atomic_dec_and_test(&event->event_limit)) {
+			ret = 1;
+			event->pending_kill = POLL_HUP;
 
-		perf_event_disable_inatomic(event);
+			perf_event_disable_inatomic(event);
+		}
 	}
 
 	READ_ONCE(event->overflow_handler)(event, data, regs);
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 2831480c63a2..4e7c728569a8 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -21,6 +21,9 @@ static void perf_output_wakeup(struct perf_output_handle *handle)
 {
 	atomic_set(&handle->rb->poll, POLLIN);
 
+	if (handle->event->attr.signal_on_wakeup)
+		handle->event->pending_kill = POLL_IN;
+
 	handle->event->pending_wakeup = 1;
 	irq_work_queue(&handle->event->pending);
 }
-- 
2.12.2

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [RFC PATCH v2 2/2] kernel/events: Add option to enable counting sideband events in wakeup_events
  2017-06-13 11:33 [RFC PATCH v2 0/2] Notifications for perf sideband events Naveen N. Rao
  2017-06-13 11:33 ` [RFC PATCH v2 1/2] kernel/events: Add option to notify through signals on wakeup Naveen N. Rao
@ 2017-06-13 11:33 ` Naveen N. Rao
  1 sibling, 0 replies; 5+ messages in thread
From: Naveen N. Rao @ 2017-06-13 11:33 UTC (permalink / raw)
  To: Peter Zijlstra, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar
  Cc: linux-kernel

Many sideband events are interesting by themselves. When profiling only
for sideband events, it is useful to be able to control process wakeup
(wakeup_events) based on sideband events alone. Add a new option
'count_sb_events' to do the same.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 include/uapi/linux/perf_event.h |  3 ++-
 kernel/events/core.c            |  2 +-
 kernel/events/ring_buffer.c     | 13 +++++++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index e5810b1d74a4..ab4dc9a02151 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -346,7 +346,8 @@ struct perf_event_attr {
 				write_backward :  1, /* Write ring buffer from end to beginning */
 				namespaces     :  1, /* include namespaces data */
 				signal_on_wakeup : 1, /* send signal on wakeup */
-				__reserved_1   : 34;
+				count_sb_events  : 1, /* wakeup_events also counts sideband events */
+				__reserved_1   : 33;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 73ad30e124e5..1d8cd3911356 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5955,7 +5955,7 @@ void perf_output_sample(struct perf_output_handle *handle,
 		}
 	}
 
-	if (!event->attr.watermark) {
+	if (!event->attr.count_sb_events && !event->attr.watermark) {
 		int wakeup_events = event->attr.wakeup_events;
 
 		if (wakeup_events) {
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 4e7c728569a8..f43a6081141f 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -197,6 +197,19 @@ __perf_output_begin(struct perf_output_handle *handle,
 	 * none of the data stores below can be lifted up by the compiler.
 	 */
 
+	if (event->attr.count_sb_events && !event->attr.watermark) {
+		int wakeup_events = event->attr.wakeup_events;
+
+		if (wakeup_events) {
+			int events = local_inc_return(&rb->events);
+
+			if (events >= wakeup_events) {
+				local_sub(wakeup_events, &rb->events);
+				local_inc(&rb->wakeup);
+			}
+		}
+	}
+
 	if (unlikely(head - local_read(&rb->wakeup) > rb->watermark))
 		local_add(rb->watermark, &rb->wakeup);
 
-- 
2.12.2

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH v2 1/2] kernel/events: Add option to notify through signals on wakeup
  2017-06-13 11:33 ` [RFC PATCH v2 1/2] kernel/events: Add option to notify through signals on wakeup Naveen N. Rao
@ 2017-06-13 14:45   ` Jiri Olsa
  2017-06-13 15:16     ` Naveen N. Rao
  0 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2017-06-13 14:45 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel

On Tue, Jun 13, 2017 at 05:03:42PM +0530, Naveen N. Rao wrote:

SNIP

> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index b1c0b187acfe..e5810b1d74a4 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -345,7 +345,8 @@ struct perf_event_attr {
>  				context_switch :  1, /* context switch data */
>  				write_backward :  1, /* Write ring buffer from end to beginning */
>  				namespaces     :  1, /* include namespaces data */
> -				__reserved_1   : 35;
> +				signal_on_wakeup : 1, /* send signal on wakeup */
> +				__reserved_1   : 34;
>  
>  	union {
>  		__u32		wakeup_events;	  /* wakeup every n events */
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 6c4e523dc1e2..73ad30e124e5 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7362,12 +7362,14 @@ static int __perf_event_overflow(struct perf_event *event,
>  	 * events
>  	 */
>  
> -	event->pending_kill = POLL_IN;
> -	if (events && atomic_dec_and_test(&event->event_limit)) {
> -		ret = 1;
> -		event->pending_kill = POLL_HUP;
> +	if (!event->attr.signal_on_wakeup) {
> +		event->pending_kill = POLL_IN;
> +		if (events && atomic_dec_and_test(&event->event_limit)) {
> +			ret = 1;
> +			event->pending_kill = POLL_HUP;
>  
> -		perf_event_disable_inatomic(event);
> +			perf_event_disable_inatomic(event);
> +		}
>  	}
>  
>  	READ_ONCE(event->overflow_handler)(event, data, regs);
> diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> index 2831480c63a2..4e7c728569a8 100644
> --- a/kernel/events/ring_buffer.c
> +++ b/kernel/events/ring_buffer.c
> @@ -21,6 +21,9 @@ static void perf_output_wakeup(struct perf_output_handle *handle)
>  {
>  	atomic_set(&handle->rb->poll, POLLIN);
>  
> +	if (handle->event->attr.signal_on_wakeup)
> +		handle->event->pending_kill = POLL_IN;
> +

since it's signal_on_wakeup, should we also send POLL_HUP for
perf_event_wakeup calls from perf_event_exit_event?

jirka

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH v2 1/2] kernel/events: Add option to notify through signals on wakeup
  2017-06-13 14:45   ` Jiri Olsa
@ 2017-06-13 15:16     ` Naveen N. Rao
  0 siblings, 0 replies; 5+ messages in thread
From: Naveen N. Rao @ 2017-06-13 15:16 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel

Hi Jiri,

On 2017/06/13 04:45PM, Jiri Olsa wrote:
> On Tue, Jun 13, 2017 at 05:03:42PM +0530, Naveen N. Rao wrote:
> 
> SNIP
> 
> > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> > index b1c0b187acfe..e5810b1d74a4 100644
> > --- a/include/uapi/linux/perf_event.h
> > +++ b/include/uapi/linux/perf_event.h
> > @@ -345,7 +345,8 @@ struct perf_event_attr {
> >  				context_switch :  1, /* context switch data */
> >  				write_backward :  1, /* Write ring buffer from end to beginning */
> >  				namespaces     :  1, /* include namespaces data */
> > -				__reserved_1   : 35;
> > +				signal_on_wakeup : 1, /* send signal on wakeup */
> > +				__reserved_1   : 34;
> >  
> >  	union {
> >  		__u32		wakeup_events;	  /* wakeup every n events */
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 6c4e523dc1e2..73ad30e124e5 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -7362,12 +7362,14 @@ static int __perf_event_overflow(struct perf_event *event,
> >  	 * events
> >  	 */
> >  
> > -	event->pending_kill = POLL_IN;
> > -	if (events && atomic_dec_and_test(&event->event_limit)) {
> > -		ret = 1;
> > -		event->pending_kill = POLL_HUP;
> > +	if (!event->attr.signal_on_wakeup) {
> > +		event->pending_kill = POLL_IN;
> > +		if (events && atomic_dec_and_test(&event->event_limit)) {
> > +			ret = 1;
> > +			event->pending_kill = POLL_HUP;
> >  
> > -		perf_event_disable_inatomic(event);
> > +			perf_event_disable_inatomic(event);
> > +		}
> >  	}
> >  
> >  	READ_ONCE(event->overflow_handler)(event, data, regs);
> > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> > index 2831480c63a2..4e7c728569a8 100644
> > --- a/kernel/events/ring_buffer.c
> > +++ b/kernel/events/ring_buffer.c
> > @@ -21,6 +21,9 @@ static void perf_output_wakeup(struct perf_output_handle *handle)
> >  {
> >  	atomic_set(&handle->rb->poll, POLLIN);
> >  
> > +	if (handle->event->attr.signal_on_wakeup)
> > +		handle->event->pending_kill = POLL_IN;
> > +
> 
> since it's signal_on_wakeup, should we also send POLL_HUP for
> perf_event_wakeup calls from perf_event_exit_event?

Yes, I suspect so (though I'm open to changing the name ;)
I haven't handled POLL_HUP properly in this patch, including 
IOC_REFRESH. I will look into that next.

I wanted to know if the overall approach is fine -- between RFC v1 vs.  
this version.

Thanks for the review!
- Naveen

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-06-13 15:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-13 11:33 [RFC PATCH v2 0/2] Notifications for perf sideband events Naveen N. Rao
2017-06-13 11:33 ` [RFC PATCH v2 1/2] kernel/events: Add option to notify through signals on wakeup Naveen N. Rao
2017-06-13 14:45   ` Jiri Olsa
2017-06-13 15:16     ` Naveen N. Rao
2017-06-13 11:33 ` [RFC PATCH v2 2/2] kernel/events: Add option to enable counting sideband events in wakeup_events Naveen N. Rao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).