Re: [PATCH] de_thread: eliminate unneccessary sighand locking

[Ingo Molnar]

* Roland McGrath <roland@xxxxxxxxxx> wrote:

> That is not the scenario.  Something else must hold tasklist_lock 
> while acquiring ourtask->sighand->siglock, but need not hold 
> tasklist_lock throughout.  Someone can be holding oldsighand->lock but 
> not not tasklist_lock, at the time we lock tasklist_lock.  So like I 
> said, we need to hold oldsighand->siglock until the switch has been 
> done.
> 
> It's possible that no such scenarios exist, but I'd really have to 
> check on that.  My recollection is that there might be some.

i have checked it when acking the patch and it does not seem we do that 
anywhere in the kernel. It would also be dangerous as per Oleg's 
observations, unless something like this is done:

	read_lock(&tasklist_lock);
	p = find_task_by_pid(pid);
	task_lock(p);
	spin_lock(&p->sighand->siglock);
	read_unlock(&tasklist_lock);

	...

do you know any such code?

> > Just for my education, could you please point me to the existed example?
> 
> It would require some auditing to hunt down whether they exist or 
> don't. To make the change you suggest would require complete 
> confidence none exist.

i have manually reviewed every .[ch] file that uses tasklist_lock, 
siglock and lock_task:

  fs/proc/array.c
  fs/exec.c
  kernel/ptrace.c
  kernel/fork.c
  kernel/exit.c
  kernel/sys.c
  include/linux/sched.h
  security/selinux/hooks.c

can other valid locking variations exist, other than the one i outlined 
above?

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