Messages in this thread | | | Subject | Re: [PATCH 2/2] CRED: Fix __task_cred()'s lockdep check and banner comment | From | (Eric W. Biederman) | Date | Thu, 05 Aug 2010 13:13:03 -0700 |
| |
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Thu, Aug 5, 2010 at 12:19 AM, Eric W. Biederman > <ebiederm@xmission.com> 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.
It is about the list traversal. In the process group case it is about traversing the pid->tasks[PIDTYPE_PGID] hlist, which is also protected by the tasklist_lock.
> 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 > spinlocks.
I think it is totally reasonable to add a per pid lock, that would protect the pid->task[...] hlist. That would make things clearer and finer grained without a lot of effort. Just a little more struct pid bloat, and a little extra care in fork, when we add to those lists.
Even with the per-pgrp lock we still need a lock on the global process list for the kill -KILL -1 case. Which suggests that tasklist_lock is still needed for part of kill_something_info.
Eric -- 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/
| |