Messages in this thread | | | Date | Tue, 8 Jul 2014 08:50:05 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH tip/core/rcu 03/17] signal: Explain local_irq_save() call |
| |
On Tue, Jul 08, 2014 at 05:01:51PM +0800, Lai Jiangshan wrote: > On 07/08/2014 06:38 AM, Paul E. McKenney wrote: > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> > > > > The explicit local_irq_save() in __lock_task_sighand() is needed to avoid > > a potential deadlock condition, as noted in a841796f11c90d53 (signal: > > align __lock_task_sighand() irq disabling and RCU). However, someone > > reading the code might be forgiven for concluding that this separate > > local_irq_save() was completely unnecessary. This commit therefore adds > > a comment referencing the shiny new block comment on rcu_read_unlock(). > > > > Reported-by: Oleg Nesterov <oleg@redhat.com> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > > Acked-by: Oleg Nesterov <oleg@redhat.com> > > --- > > kernel/signal.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/kernel/signal.c b/kernel/signal.c > > index a4077e90f19f..46161e744760 100644 > > --- a/kernel/signal.c > > +++ b/kernel/signal.c > > @@ -1263,6 +1263,10 @@ struct sighand_struct *__lock_task_sighand(struct task_struct *tsk, > > struct sighand_struct *sighand; > > > > for (;;) { > > + /* > > + * Disable interrupts early to avoid deadlocks. > > + * See rcu_read_unlock comment header for details. > > + */ > > A pair of brackets are missing here: rcu_read_unlock() > after that, please add: > > Reviewed-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Good point, added the "()" and your Reviewed-by.
> It reminds me that I should keep my effort to solve the deadlock > problem where rcu_read_unlock() is overlapped with schedule locks.
That would be good! I vaguely remember an earlier patch of yours that Steven Rostedt gave feedback on, but have not been able to locate either email.
Thanx, Paul
> > local_irq_save(*flags); > > rcu_read_lock(); > > sighand = rcu_dereference(tsk->sighand); >
| |