Messages in this thread | | | Date | Mon, 5 May 2014 21:55:09 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH] signal: Simplify __lock_task_sighand() |
| |
On 05/05, Oleg Nesterov wrote: > > Yes, but please consider the cleanup below, on top of your change. > > This is subjective of course, but imho the code looks better without > the extra unlock/restore inside the loop. > > ------------------------------------------------------------------------------- > Subject: [PATCH] signal: Simplify __lock_task_sighand() > > __lock_task_sighand() does local_irq_save() to prevent the potential > deadlock, we can use preempt_disable() with the same effect. And in > this case we can do preempt_disable/enable + rcu_read_lock/unlock only > once outside of the main loop and simplify the code. Also shaves 112 > bytes from signal.o. > > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > --- > kernel/signal.c | 31 +++++++++++++------------------ > 1 files changed, 13 insertions(+), 18 deletions(-) > > diff --git a/kernel/signal.c b/kernel/signal.c > index 4368370..03a0fd4 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -1260,30 +1260,25 @@ struct sighand_struct *__lock_task_sighand(struct task_struct *tsk, > unsigned long *flags) > { > struct sighand_struct *sighand; > - > + /* > + * We are going to do rcu_read_unlock() under spin_lock_irqsave(). > + * Make sure we can not be preempted after rcu_read_unlock(), see ^^^^^^^^^^^^^^^ Argh, typo in comment. I meant rcu_read_lock() of course.
I'll send v2 tomorrow unless you dislike this change.
Oleg.
| |