[PATCH] coredump: zap_threads: comments && use while_each_thread() - Kernel

This is a discussion on [PATCH] coredump: zap_threads: comments && use while_each_thread() - Kernel ; No changes in fs/exec.o The for_each_process() loop in zap_threads() is very subtle, it is not clear why we don't race with fork/exit/exec. Add the fat comment. Also, change the code to use while_each_thread(). Signed-off-by: Oleg Nesterov --- 26-rc2/fs/exec.c~3_CDUMP_COMMENTS 2008-05-26 19:37:35.000000000 ...

+ Reply to Thread
Results 1 to 2 of 2

Thread: [PATCH] coredump: zap_threads: comments && use while_each_thread()

  1. [PATCH] coredump: zap_threads: comments && use while_each_thread()

    No changes in fs/exec.o

    The for_each_process() loop in zap_threads() is very subtle, it is not clear
    why we don't race with fork/exit/exec. Add the fat comment.

    Also, change the code to use while_each_thread().

    Signed-off-by: Oleg Nesterov

    --- 26-rc2/fs/exec.c~3_CDUMP_COMMENTS 2008-05-26 19:37:35.000000000 +0400
    +++ 26-rc2/fs/exec.c 2008-05-31 20:05:21.000000000 +0400
    @@ -1512,7 +1512,7 @@ static void zap_process(struct task_stru
    sigaddset(&t->pending.signal, SIGKILL);
    signal_wake_up(t, 1);
    }
    - } while ((t = next_thread(t)) != start);
    + } while_each_thread(start, t);
    }

    static inline int zap_threads(struct task_struct *tsk, struct mm_struct *mm,
    @@ -1534,7 +1534,36 @@ static inline int zap_threads(struct tas

    if (atomic_read(&mm->mm_users) == mm->core_waiters + 1)
    goto done;
    -
    + /*
    + * We should find and kill all tasks which use this mm, and we should
    + * count them correctly into mm->core_waiters. We don't take tasklist
    + * lock, but this is safe wrt:
    + *
    + * fork:
    + * None of sub-threads can fork after zap_process(leader). All
    + * processes which were created before this point should be
    + * visible to zap_threads() because copy_process() adds the new
    + * process to the tail of init_task.tasks list, and lock/unlock
    + * of ->siglock provides a memory barrier.
    + *
    + * do_exit:
    + * The caller holds mm->mmap_sem. This means that the task which
    + * uses this mm can't pass exit_mm(), so it can't exit or clear
    + * its ->mm.
    + *
    + * de_thread:
    + * It does list_replace_rcu(&leader->tasks, &current->tasks),
    + * we must see either old or new leader, this does not matter.
    + * However, it can change p->sighand, so lock_task_sighand(p)
    + * must be used. Since p->mm != NULL and we hold ->mmap_sem
    + * it can't fail.
    + *
    + * Note also that "g" can be the old leader with ->mm == NULL
    + * and already unhashed and thus removed from ->thread_group.
    + * This is OK, __unhash_process()->list_del_rcu() does not
    + * clear the ->next pointer, we will find the new leader via
    + * next_thread().
    + */
    rcu_read_lock();
    for_each_process(g) {
    if (g == tsk->group_leader)
    @@ -1544,17 +1573,13 @@ static inline int zap_threads(struct tas
    do {
    if (p->mm) {
    if (p->mm == mm) {
    - /*
    - * p->sighand can't disappear, but
    - * may be changed by de_thread()
    - */
    lock_task_sighand(p, &flags);
    zap_process(p);
    unlock_task_sighand(p, &flags);
    }
    break;
    }
    - } while ((p = next_thread(p)) != g);
    + } while_each_thread(g, p);
    }
    rcu_read_unlock();
    done:

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] coredump: zap_threads: comments && use while_each_thread()

    Acked-by: Roland McGrath
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread