Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 4 Feb 2021 14:33:32 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Dmitry Vyukov <dvyukov@google.com>
Cc:     Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Will Deacon <will@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Matt Morehouse <mascasa@google.com>
Subject: Re: Process-wide watchpoints
Message-ID: <YBv3rAT566k+6zjg@hirez.programming.kicks-ass.net>
References: <CACT4Y+ZsKXfAxrzJGQc5mJ+QiP5sAw7zKWtciS+07qZzSf33mw@mail.gmail.com>
 <CACT4Y+YeRtOTsMQ8xxZg-=nbv+yuJvYYhBErT46M8jtSHmiw6g@mail.gmail.com>
 <YBqXPmbpXf4hnlj3@hirez.programming.kicks-ass.net>
 <CACT4Y+a-9kqX0ZkNz-ygib+ERn41HVo_8Wx6oYMQmPjTC06j7g@mail.gmail.com>
 <YBqnAYVdNM4uyGny@hirez.programming.kicks-ass.net>
 <CACT4Y+btOt5QFKH9Q=81EnpDHoidJUHE2s0oZ8v65t-b__awuw@mail.gmail.com>
 <YBvAsku9OWM7KUno@hirez.programming.kicks-ass.net>
 <CACT4Y+ZLSyVMkPfh3PftEWKC1kC+o1XLxo_o6i4BiyRuPig27g@mail.gmail.com>
 <YBvj6eJR/DY2TsEB@hirez.programming.kicks-ass.net>
 <CACT4Y+a17L2pUY1kkRB_v_y3P_sbMpSLb6rVfXmGM7LkbAvj5Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CACT4Y+a17L2pUY1kkRB_v_y3P_sbMpSLb6rVfXmGM7LkbAvj5Q@mail.gmail.com>
Precedence: bulk

On Thu, Feb 04, 2021 at 01:53:59PM +0100, Dmitry Vyukov wrote:
> On Thu, Feb 4, 2021 at 1:09 PM Peter Zijlstra <peterz@infradead.org> wrote:

> > What do we do then? The advantage of IOC_REFRESH is that it disables the
> > event until it gets explicitly re-armed, avoiding recursion issues etc.
> > Do you want those semantics? If so, we'd need to have IOC_REFRESH find
> > the actual event for the current task, which should be doable I suppose.
> 
> Frankly, I don't know. I didn't use it in my prototype, nor I fully
> understand what it's doing. Does it make sense for breakpoints?
> I see IOC_REFRESH has a check for !attr.inherit, so it will fail for
> my use case currently. I would say we just leave it as is for now.

Well, the way it works is that currently you set event_limit > 0. Then
each event will decrement, when we hit 0 we disable and raise a signal.

REFRESH will increment event_limit and re-enable.

This means you're guaranteed not to get another signal until you're
ready for it. It allows leaving the signal handler context to handle the
signal.

I suppose you're looking for something like this, which goes in top of
that thread_only thing.

--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -389,7 +389,8 @@ struct perf_event_attr {
 				cgroup         :  1, /* include cgroup events */
 				text_poke      :  1, /* include text poke events */
 				thread_only    :  1, /* only inherit on threads */
-				__reserved_1   : 29;
+				sigtrap        :  1, /* foo */
+				__reserved_1   : 28;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6273,6 +6273,13 @@ static void perf_pending_event_disable(s
 
 	if (cpu == smp_processor_id()) {
 		WRITE_ONCE(event->pending_disable, -1);
+
+		if (event->attr.sigtrap) {
+			atomic_inc(&event->event_limit); /* rearm */
+			send_sig_info(SIGTRAP, SEND_SIG_PRIV, current);
+			return;
+		} 
+
 		perf_event_disable_local(event);
 		return;
 	}
@@ -8936,6 +8943,7 @@ static int __perf_event_overflow(struct
 				   int throttle, struct perf_sample_data *data,
 				   struct pt_regs *regs)
 {
+	perf_overflow_handler_t ovf;
 	int events = atomic_read(&event->event_limit);
 	int ret = 0;
 
@@ -8961,7 +8969,15 @@ static int __perf_event_overflow(struct
 		perf_event_disable_inatomic(event);
 	}
 
-	READ_ONCE(event->overflow_handler)(event, data, regs);
+	ovf = READ_ONCE(event->overflow_handler);
+#ifdef CONFIG_RETPOLINE
+	if (ovf == perf_event_output_forward) {
+		perf_event_output_forward(event, data, regs);
+	} else if (ovf == perf_event_output_backward) {
+		perf_event_output_backward(event, data, regs);
+	} else
+#endif
+		ovf(event, data, regs);
 
 	if (*perf_event_fasync(event) && event->pending_kill) {
 		event->pending_wakeup = 1;
@@ -11281,6 +11297,9 @@ perf_event_alloc(struct perf_event_attr
 
 	event->state		= PERF_EVENT_STATE_INACTIVE;
 
+	if (event->attr.sigtrap)
+		event->event_limit = ATOMIC_INIT(1);
+
 	if (task) {
 		event->attach_state = PERF_ATTACH_TASK;
 		/*
@@ -11556,6 +11575,9 @@ static int perf_copy_attr(struct perf_ev
 	if (attr->thread_only && !attr->inherit)
 		return -EINVAL;
 
+	if (attr->sigtrap && attr->inherit && !attr->thread_only)
+		return -EINVAL;
+
 out:
 	return ret;