[lkml]   [2010]   [Aug]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 2/2] CRED: Fix __task_cred()'s lockdep check and banner comment
    On Thu, Aug 5, 2010 at 12:19 AM, Eric W. Biederman
    <> wrote:
    > No.  When we send a signal to multiple processes it needs to be an
    > atomic operation so that kill -KILL -pgrp won't let processes escape.
    > It is what posix specifies, it is what real programs expect, and it
    > is the useful semantic in userspace.

    Ok. However, in that case, it's not really about the whole list
    traversal, it's a totally separate thing, and it's really sad that we
    end up using the (rather hot) tasklist_lock for something like that.
    With the dcache/inode locks basically going away, I think
    tasklist_lock ends up being one of the few hot locks left.

    Wouldn't it be much nicer to:
    - make it clear that all the "real" signal locking can rely on RCU
    - use a separate per-pgrp lock that ends up being the one that gives
    the signal _semantic_ meaning?

    That would automatically document why we get the lock too, which
    certainly isn't clear from the code as-is.

    The per-pgrp lock might be something as simple as a silly hash that
    just spreads out the process groups over some random number of simple

    > With the tasklist_lock the rule in these functions is that the caller
    > will take the lock, so we probably make the rule the caller should
    > take the lock in the same scenarios for the rcu_read_lock(). Aka just
    > say:
    > read_lock(&tasklist_lock);
    > rcu_read_lock();
    > everywhere, that today we say just:
    > read_lock(&tasklist_lock);

    I agree that we probably should have done that originally, in order to
    not have these bugs show up later. However, I don't think it makes
    sense any more, especially not if tasklist_lock isn't even a "real"
    lock from a kernel internal consistency standpoint, but has a totally
    secondary meaning.

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

     \ /
      Last update: 2010-08-05 18:25    [W:4.021 / U:0.172 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site