lkml.org 
[lkml]   [2005]   [Jun]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] de_thread: eliminate unneccessary sighand locking

* Roland McGrath <roland@redhat.com> wrote:

> > do you know any such code?
>
> I was thinking it would look more like:
>
> read_lock(&tasklist_lock);
> p = find_task_by_pid(pid);
> get_task_struct(p);
> read_unlock(&tasklist_lock);
> ...
> spin_lock_irq(&p->sighand->siglock);
> ...

the amount of potentially affected code (assuming all the locking is
done in a single .[ch] file) is even smaller:

kernel/posix-cpu-timers.c
kernel/exit.c
kernel/posix-timers.c
include/linux/sched.h

checked these, we dont seem to do that.

> I am not sure whether such code exists or not. It won't look quite
> like that, in that the siglock use may be far away from the
> extraction. There are things that store pointers like that (like
> real_timer.data, and posix_timers stuff). But it may well be that all
> those places take tasklist_lock before using the saved task_struct, in
> which case it's fine. (Anything doing signals stuff usually needs
> tasklist_lock anyway in case it has to traverse the thread group.)

this reminds me about the patch below: it gets rid of tasklist_lock use
in the POSIX timer signal delivery critical path.

> You mean those are the files that use all three of those, or what?
> That's clearly not the complete list of siglock uses. Any code using
> siglock needs to be grokked adequately to see if tasklist_lock is
> always held around looking at ->sighand.

yeah.

Ingo

---

dont use the tasklist_lock in the POSIX timer-delivery critical path.

Signed-off-by: Ingo Molnar <mingo@elte.hu>

--- linux/kernel/signal.c.orig
+++ linux/kernel/signal.c
@@ -1384,7 +1384,7 @@ send_sigqueue(int sig, struct sigqueue *
* going away or changing from under us.
*/
BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
- read_lock(&tasklist_lock);
+ task_lock(p);
spin_lock_irqsave(&p->sighand->siglock, flags);

if (unlikely(!list_empty(&q->list))) {
@@ -1411,7 +1411,7 @@ send_sigqueue(int sig, struct sigqueue *

out:
spin_unlock_irqrestore(&p->sighand->siglock, flags);
- read_unlock(&tasklist_lock);
+ task_unlock(p);
return(ret);
}

-
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/
\
 
 \ /
  Last update: 2005-06-28 09:29    [W:0.170 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site