[lkml]   [2012]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Subject[PATCH 0/3] exec_id/exit_signal fixes
    Andrew, these changes are orthogonal to
    "CLONE_PARENT shouldn't allow to set ->exit_signal" in -mm.

    Please review/comment. The patches are really simple I hope,
    but with or without them I do not understand eligible_child().

    /* Wait for all children (clone and not) if __WALL is set;
    * otherwise, wait for clone children *only* if __WCLONE is
    * set; otherwise, wait for non-clone children *only*. (Note:
    * A "clone" child here is one that reports to its parent
    * using a signal other than SIGCHLD.) */
    if (((p->exit_signal != SIGCHLD) ^ !!(wo->wo_flags & __WCLONE))
    && !(wo->wo_flags & __WALL))
    return 0;

    OK, but exec/exit can change ->exit_signal "in between".

    For example. The parent clones the !SIGCHLD child and does
    do_wait(__WCLONE). The child execs. The parent still sleeps until
    child exits but do_wait() returns ECHILD. And more, this also
    depends on who does exec in mt case.

    Or the parent execs. In this case eligible_child() depends on when
    the child exits, before or after exec.

    Anyway, the current check in exit_notify() is wrong, 2/2 tries to
    fix this. But both patches add the (hopefully minor) user-visible
    changes wrt eligible_child().

    Really, I think de_thread() should set ->exit_signal = SIGCHLD for
    each child. This looks simple and understandable, and with the
    CLONE_PARENT change above we can kill self_exec_id/parent_exec_id.

    May be de_thread() should also do __wake_up_parent() if it changes
    ->exit_signal. Or perhaps we should change eligible_child() to check
    p->cloned_with_not_SIGCHLD. I dunno. I hope this doesn't matter
    and nobody cares.

    OTOH, I never understood why we should reset ->exit_signal if the
    child execs but the parent is still the same.

    3/3 is offtopic minor cleanup. Imho setmax_mm_hiwater_rss() in
    de_thread() looks strange.


     \ /
      Last update: 2012-03-19 17:13    [W:0.020 / U:30.916 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site