On Thu, Mar 25, 2021 at 09:14:39AM +0100, Marco Elver wrote:
On Wed, Mar 24, 2021 at 12:24PM +0100, Marco Elver wrote: [...]
diff --git a/kernel/events/core.c b/kernel/events/core.c index b6434697c516..1e4c949bf75f 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -6391,6 +6391,17 @@ void perf_event_wakeup(struct perf_event *event) } } +static void perf_sigtrap(struct perf_event *event) +{
- struct kernel_siginfo info;
I think we need to add something like this here:
diff --git a/kernel/events/core.c b/kernel/events/core.c index 4b82788fbaab..4fcd6b45ce66 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -6395,6 +6395,13 @@ static void perf_sigtrap(struct perf_event *event) { struct kernel_siginfo info;
- /*
* This irq_work can race with an exiting task; bail out if sighand has* already been released in release_task().*/- if (!current->sighand)
return;- clear_siginfo(&info); info.si_signo = SIGTRAP; info.si_code = TRAP_PERF;
Urgh.. I'm not entirely sure that check is correct, but I always forget the rules with signal. It could be we ought to be testing PF_EXISTING instead.
But also, I think Jiri Olsa was going to poke around here because all of this is broken on PREEMPT_RT. IIRC the plan was to add yet another stage to the construct. So where today we have:
<NMI> irq_work_queue() </NMI> ... <IRQ> perf_pending_event() </IRQ>
(and we might already have a problem on some architectures where there can be significant time between these due to not having arch_irq_work_raise(), so ideally we ought to double check current in your case)
The idea was, I think to add a task_work(), such that we get:
<NMI> irq_work_queue() </NMI> ... <IRQ> perf_pending_event() task_work_add() </IRQ>
<ret-to-user> run_task_work() ... kill_fasync();