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 <o...@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
