Re: [PATCH v3 6/7] perf: Rename perf_event_exit_task(.child)

[Ravi Bangoria]

On 10-Mar-25 8:17 PM, Peter Zijlstra wrote:
> On Mon, Mar 10, 2025 at 04:38:36PM +0530, Ravi Bangoria wrote:
>> Hi Peter,
>>
>> On 08-Mar-25 1:03 AM, Peter Zijlstra wrote:
>>> The task passed to perf_event_exit_task() is not a child, it is
>>> current. Fix this confusing naming, since much of the rest of the code
>>> also relies on it being current.
>>>
>>> Specifically, both exec() and exit() callers use it with current as
>>> the argument.
>>
>> ...
>>
>>> -static void perf_event_exit_task_context(struct task_struct *child, bool exit)
>>> +static void perf_event_exit_task_context(struct task_struct *task, bool exit)
>>>  {
>>> -	struct perf_event_context *child_ctx, *clone_ctx = NULL;
>>> +	struct perf_event_context *ctx, *clone_ctx = NULL;
>>>  	struct perf_event *child_event, *next;
>>>  
>>> -	WARN_ON_ONCE(child != current);
>>> +	WARN_ON_ONCE(task != current);
>>
>> exec() codepath (i.e. copy_process()) passes child pointer, not 'current'.
> 
> I am confused, this not a new warning.

Right, However the WARN was present only in perf_event_exit_task_context()
before merging it with perf_event_free_task() (patch #5). And
perf_event_free_task() is getting called for child task.

> Also, copy_process() is clone(), exec() is another code path.

My bad. I meant clone() code path:

  copy_process()
    p = dup_task_struct(current);
    perf_event_init_task(p);
      perf_event_free_task(p);
        perf_event_exit_task_context(p);
          WARN_ON_ONCE(task != current);

Another one:

  copy_process()
    p = dup_task_struct(current);
    ...
    bad_fork_cleanup_perf:
      perf_event_free_task(p);
        perf_event_exit_task_context(p);
          WARN_ON_ONCE(task != current);

Or am I missing something?

Thanks,
Ravi