| Message ID | 20240731095017.1560516-1-liaochang1@huawei.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Masami Hiramatsu |
| Headers | show |
| Series | uprobes: Remove redundant spinlock in uprobe_deny_signal | expand |
On 07/31, Liao Chang wrote: > > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -1979,9 +1979,7 @@ bool uprobe_deny_signal(void) > WARN_ON_ONCE(utask->state != UTASK_SSTEP); > > if (task_sigpending(t)) { > - spin_lock_irq(&t->sighand->siglock); > clear_tsk_thread_flag(t, TIF_SIGPENDING); > - spin_unlock_irq(&t->sighand->siglock); Agreed, in this case ->siglock buys nothing, another signal can come right after spin_unlock(). Acked-by: Oleg Nesterov <oleg@redhat.com>
On Wed, 31 Jul 2024 16:12:32 +0200 Oleg Nesterov <oleg@redhat.com> wrote: > On 07/31, Liao Chang wrote: > > > > --- a/kernel/events/uprobes.c > > +++ b/kernel/events/uprobes.c > > @@ -1979,9 +1979,7 @@ bool uprobe_deny_signal(void) > > WARN_ON_ONCE(utask->state != UTASK_SSTEP); > > > > if (task_sigpending(t)) { > > - spin_lock_irq(&t->sighand->siglock); > > clear_tsk_thread_flag(t, TIF_SIGPENDING); > > - spin_unlock_irq(&t->sighand->siglock); > > Agreed, in this case ->siglock buys nothing, another signal can come > right after spin_unlock(). > > Acked-by: Oleg Nesterov <oleg@redhat.com> > > Looks good to me. Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Peter, can you pick this? Thanks,
On Tue, 22 Oct 2024 11:22:22 +0900 Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > On Wed, 31 Jul 2024 16:12:32 +0200 > Oleg Nesterov <oleg@redhat.com> wrote: > > > On 07/31, Liao Chang wrote: > > > > > > --- a/kernel/events/uprobes.c > > > +++ b/kernel/events/uprobes.c > > > @@ -1979,9 +1979,7 @@ bool uprobe_deny_signal(void) > > > WARN_ON_ONCE(utask->state != UTASK_SSTEP); > > > > > > if (task_sigpending(t)) { > > > - spin_lock_irq(&t->sighand->siglock); > > > clear_tsk_thread_flag(t, TIF_SIGPENDING); > > > - spin_unlock_irq(&t->sighand->siglock); > > > > Agreed, in this case ->siglock buys nothing, another signal can come > > right after spin_unlock(). > > > > Acked-by: Oleg Nesterov <oleg@redhat.com> > > > > > > Looks good to me. > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Peter, can you pick this? Oops, there is v3. https://lore.kernel.org/linux-trace-kernel/20240815014629.2685155-2-liaochang1@huawei.com/ > > Thanks, > > -- > Masami Hiramatsu (Google) <mhiramat@kernel.org>
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 73cc47708679..76a51a1f51e2 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -1979,9 +1979,7 @@ bool uprobe_deny_signal(void) WARN_ON_ONCE(utask->state != UTASK_SSTEP); if (task_sigpending(t)) { - spin_lock_irq(&t->sighand->siglock); clear_tsk_thread_flag(t, TIF_SIGPENDING); - spin_unlock_irq(&t->sighand->siglock); if (__fatal_signal_pending(t) || arch_uprobe_xol_was_trapped(t)) { utask->state = UTASK_SSTEP_TRAPPED;
Since clearing a bit in thread_info is an atomic operation, the spinlock is redundant and can be removed, reducing lock contention is good for performance. Signed-off-by: Liao Chang <liaochang1@huawei.com> --- kernel/events/uprobes.c | 2 -- 1 file changed, 2 deletions(-)