lkml.org 
[lkml]   [2022]   [Aug]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC v2] perf: Rewrite core context handling
    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.

    \
     
     \ /
      Last update: 2022-08-22 18:53    [W:3.156 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site