Re: [RFC v2] perf: Rewrite core context handling

[Ravi Bangoria]

On 22-Aug-22 10:22 PM, Peter Zijlstra wrote:
> On Tue, Aug 02, 2022 at 11:41:42AM +0530, Ravi Bangoria wrote:
> 
>>> @@ -12358,58 +12374,14 @@ SYSCALL_DEFINE5(perf_event_open,
>>>  			goto err_context;
>>>  	}
>>>  
>>> -	event_file = anon_inode_getfile("[perf_event]", &perf_fops, event, f_flags);
>>> -	if (IS_ERR(event_file)) {
>>> -		err = PTR_ERR(event_file);
>>> -		event_file = NULL;
>>> -		goto err_context;
>>> -	}
>>> -
>>> -	if (task) {
>>> -		err = down_read_interruptible(&task->signal->exec_update_lock);
>>> -		if (err)
>>> -			goto err_file;
>>> -
>>> -		/*
>>> -		 * We must hold exec_update_lock across this and any potential
>>> -		 * perf_install_in_context() call for this new event to
>>> -		 * serialize against exec() altering our credentials (and the
>>> -		 * perf_event_exit_task() that could imply).
>>> -		 */
>>> -		err = -EACCES;
>>> -		if (!perf_check_permission(&attr, task))
>>> -			goto err_cred;
>>> -	}
>>> -
>>> -	if (ctx->task == TASK_TOMBSTONE) {
>>> -		err = -ESRCH;
>>> -		goto err_locked;
>>> -	}
>>
>> I think we need to keep (ctx->task == TASK_TOMBSTONE) check?
> 
> I think so too; in fact the code I have still has it, perhaps it was
> there write before this patch?
> 
>>> -
>>>  	if (!perf_event_validate_size(event)) {
>>>  		err = -E2BIG;
>>> -		goto err_locked;
>>> -	}
>>> -
>>> -	if (!task) {
>>> -		/*
>>> -		 * Check if the @cpu we're creating an event for is online.
>>> -		 *
>>> -		 * We use the perf_cpu_context::ctx::mutex to serialize against
>>> -		 * the hotplug notifiers. See perf_event_{init,exit}_cpu().
>>> -		 */
>>> -		struct perf_cpu_context *cpuctx =
>>> -			container_of(ctx, struct perf_cpu_context, ctx);
>>> -
>>> -		if (!cpuctx->online) {
>>> -			err = -ENODEV;
>>> -			goto err_locked;
>>> -		}
>>> +		goto err_context;
>>
>> Why did you remove this hunk? We should confirm whether cpu is online or not
>> before creating event. No?
> 
> Idem.
> 
> Perhaps it is best if we look at the end result of all these patches
> combined and then I'll fold the lot if we're in agreement and then we
> can forget about these intermediate steps.

Let me accumulate all these changes, rebase to v6.0-rc2 and send RFC v3.

Thanks,
Ravi