Re: [bug report] perf: Fix event leak upon exec and file release

[Frederic Weisbecker]

Le Mon, Sep 30, 2024 at 11:04:39AM +0200, Sebastian Andrzej Siewior a écrit :
> On 2024-09-02 23:47:08 [+0200], Frederic Weisbecker wrote:
> > Ah right.
> > 
> > So one possible fix is to possibly let the task work do the last reference
> > decrement. This would mean that freeing children events can't be always assumed
> > by the parent.
> > 
> > The below (only built tested) would do it?
> 
> This looks nice based on diffstat. I didn't look closed nor did any
> testing so far. Do we like it and wait for Dan here or?

No eventually I think the issues reported by Dan can't happen because:

pl330_free_chan_resources() <- disables preempt
-> pl330_release_channel()
   -> _free_event()
      -> perf_pending_task_sync()

That's another _free_event that is not related to perf.

Two and three:
perf_remove_from_context() <- disables preempt
__perf_event_exit_context() <- disables preempt
-> __perf_remove_from_context()
   -> perf_group_detach()
      -> perf_put_aux_event()
         -> put_event()
            -> _free_event()
               -> perf_pending_task_sync()

Four:
perf_free_event() <- disables preempt
-> perf_group_detach()
   -> perf_put_aux_event()
      -> put_event()
         -> _free_event()
            -> perf_pending_task_sync()


The put_event() calls here and above can't be the last ones. Because
if an event is attached to an aux event, detaching from it means that
aux event itself hasn't even gone through perf_event_remove_from_context().

Similarly an exiting aux event detaching the events from it means those attached
events haven't gone through perf_event_remove_from_context().

So this should be fine (famous last words). There is a might_sleep() call in
irq_work_sync() that should tell us about it.

Thanks.