lkml.org 
[lkml]   [2014]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] fork.c: copy_process(): fix cleanup WRT perf_event_free_task()
On 09/26, Sylvain 'ythier' Hitier wrote:
>
> retval = sched_fork(clone_flags, p);
> if (retval)
> // // mustn't perf_event_free_task()
> goto bad_fork_cleanup_policy;

Agreed, this is wrong. Good catch.

but, unless I missed something,

> retval = perf_event_init_task(p);
> if (retval)
> // // mustn't perf_event_free_task()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

this is not right and thus the patch is not right too.

Suppose that perf_event_init_task() -> perf_event_init_context(ctxn => 0)
succeeds and then perf_event_init_context(ctxn => 1) fails, we need
perf_event_free_task() to cleanup ->perf_event_ctxp[0].

So if perf_event_init_task() fails, we still need "goto bad_fork_cleanup_perf".

No?

Or, probably better, we need to change perf_event_init_context() to call
perf_event_free_task() on failure.

Or. We can simply move memset(child->perf_event_ctxp, 0, ...) from
perf_event_init_context() up. This reminds that we really need to cleanup
copy_process(), in particular I think it asks for the new copy_xxx() helper
which should do misc simple initializations which can't fail.

What do you think?

Oleg.



\
 
 \ /
  Last update: 2014-09-27 20:41    [W:0.115 / U:1.572 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site