Re: [PATCH] fork.c: copy_process(): fix cleanup WRT perf_event_free_task()

From: Oleg Nesterov
Date: Sat Sep 27 2014 - 14:10:56 EST

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".


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?


To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at