lkml.org 
[lkml]   [2009]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] core: allow setrlimit to non-current tasks
On 09/03, Jiri Slaby wrote:
>
> @@ -1240,6 +1240,7 @@ int setrlimit(struct task_struct *tsk, unsigned int resource,
> struct rlimit *new_rlim)
> {
> struct rlimit *old_rlim;
> + unsigned int needs_locking = !same_thread_group(tsk, current);
> int retval;

Yes, thanks for doing this, imho this optimization is worthwhile.

But I'd suggest you to add this optimization in a separate patch
because,

> + /* optimization: 'current' doesn't need locking, e.g. setrlimit */
> + if (needs_locking) {
> + /* protect tsk->signal and tsk->sighand from disappearing */
> + read_lock(&tasklist_lock);
> + if (!tsk->sighand) {
> + retval = -ESRCH;
> + goto unlock;

I should have mentioned this before, but it is not that simple.

Even if same_thread_group(tsk, current), we must not trust tsk->sighand,
it can be NULL if our subthread is dead. (well, we need ->signal, not
->sighand but this doesn't matter because they disappear simultaneously).

Actually, perhaps same_thread_group() is not needed, perhaps it is enough
to avoid tasklist in sys_setrlimit case. So, I think optimization should
do:

retval = -ESRCH;
if (tsk != current) {
read_lock(&tasklist_lock);
if (!tsk->sighand)
goto unlock;
}
unlock:
if (tsk != current)
read_unlock(&tasklist_lock);

Or, if we use same_thread_group(),

needs_locking = !same_thread_group(tsk, current);

if (!needs_locking)
tsk = current;
else {
take tasklist, check ->sighand.
}
Oleg.



\
 
 \ /
  Last update: 2009-09-03 19:49    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site