Messages in this thread Patch in this message | | | Date | Fri, 26 Sep 2014 21:06:52 +0000 | From | Sylvain 'ythier' Hitier <> | Subject | [PATCH] fork.c: copy_process(): fix cleanup WRT perf_event_free_task() |
| |
Date: Fri Sep 26 20:56:07 2014 +0000
fork.c: copy_process(): fix cleanup WRT perf_event_free_task()
Currently, in copy_process(), a failure of either sched_fork() or perf_event_init_task() does trigger perf_event_free_task()!
Moreover, the label bad_fork_cleanup_policy does more than what its name implies, because it includes perf_event_free_task()!
Let's explain the change with a grASCIIcally-enhanced kind-of-diff which provides an adequate context. // Read vertically this column // | | | | | | | | | // v v v v v v v v v { //SNIP// if (clone_flags & CLONE_THREAD) threadgroup_change_begin(current); //SNIP// #ifdef CONFIG_NUMA p->mempolicy = mpol_dup(p->mempolicy); if (IS_ERR(p->mempolicy)) { //SNIP// goto bad_fork_cleanup_threadgroup_lock; } #endif //SNIP// retval = sched_fork(clone_flags, p); if (retval) // // mustn't perf_event_free_task() goto bad_fork_cleanup_policy; retval = perf_event_init_task(p); if (retval) // // mustn't perf_event_free_task() goto bad_fork_cleanup_policy; retval = audit_alloc(p); if (retval) // // must perf_event_free_task() // @@ Hence change this way: - goto bad_fork_cleanup_policy; + goto bad_fork_cleanup_perf; //SNIP// bad_fork_cleanup_audit: audit_free(p); // // let's clean perf up // @@ Hence change this way: -bad_fork_cleanup_policy: +bad_fork_cleanup_perf: perf_event_free_task(p); // // no (longer) need to clean perf up // @@ Hence change this way: +bad_fork_cleanup_policy: #ifdef CONFIG_NUMA mpol_put(p->mempolicy); bad_fork_cleanup_threadgroup_lock: #endif if (clone_flags & CLONE_THREAD) threadgroup_change_end(current); //SNIP// }
Signed-off-by: Sylvain "ythier" Hitier <sylvain.hitier@gmail.com> --- kernel/fork.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c index 0cf9cdb..a91e47d 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1360,7 +1360,7 @@ static struct task_struct *copy_process(unsigned long clone_flags, goto bad_fork_cleanup_policy; retval = audit_alloc(p); if (retval) - goto bad_fork_cleanup_policy; + goto bad_fork_cleanup_perf; /* copy all the process information */ shm_init_task(p); retval = copy_semundo(clone_flags, p); @@ -1566,8 +1566,9 @@ bad_fork_cleanup_semundo: exit_sem(p); bad_fork_cleanup_audit: audit_free(p); -bad_fork_cleanup_policy: +bad_fork_cleanup_perf: perf_event_free_task(p); +bad_fork_cleanup_policy: #ifdef CONFIG_NUMA mpol_put(p->mempolicy); bad_fork_cleanup_threadgroup_lock: Regards, Sylvain "ythier" Hitier
-- Business is about being busy, not being rich... Lived 777 days in a Debian package => http://en.wikipedia.org/wiki/Apt,_Vaucluse There's THE room for ideals in this mechanical place!
| |