All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Mateusz Guzik <mjguzik@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] exit: change the release_task() paths to call flush_sigqueue() lockless
Date: Thu, 6 Feb 2025 17:23:02 +0100	[thread overview]
Message-ID: <20250206162301.GC5209@redhat.com> (raw)
In-Reply-To: <874j17nk11.ffs@tglx>

On 02/06, Thomas Gleixner wrote:
>
> On Wed, Feb 05 2025 at 18:51, Oleg Nesterov wrote:
> > A task can block a signal, accumulate up to RLIMIT_SIGPENDING sigqueues,
> > and exit. In this case __exit_signal()->flush_sigqueue() called with irqs
> > disabled can triger a hard lockup, see
> > https://lore.kernel.org/all/20190322114917.GC28876@redhat.com/
> >
> > Fortunately, after the recent posixtimer changes sys_timer_delete() paths
> > no longer try to clear SIGQUEUE_PREALLOC and/or free tmr->sigq, and after
> > the exiting task passes __exit_signal() lock_task_sighand() can't succeed
> > and pid_task(tmr->it_pid) will return NULL.
> >
> > This means that after __exit_signal(tsk) nobody can play with tsk->pending
> > or (if group_dead) with tsk->signal->shared_pending, so release_task() can
> > safely call flush_sigqueue() after write_unlock_irq(&tasklist_lock).
>
> I can't find a problem with that.

Ah, good.

> > Also, kill clear_tsk_thread_flag(TIF_SIGPENDING), it was never needed.
>
> I'm not entirely sure about that, but it does not hurt to clear it,
> right?

Please see v2 which documents this change in a separate patch.
Again, it is not that it is really bad, just it looks very confusing
to me and I think it can confuse other readers of this code.

> > 	- do_sigaction() can hit the similar problem
>
> Indeed, but that's a tough on to solve.

Yeah... Although I have to admit that yesterday I had a very simple
(and wrong) solution in mind ;)

Thanks!

Oleg.


  reply	other threads:[~2025-02-06 16:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-05 17:51 [PATCH 0/1] exit: change the release_task() paths to call flush_sigqueue() lockless Oleg Nesterov
2025-02-05 17:51 ` [PATCH 1/1] " Oleg Nesterov
2025-02-05 22:18   ` Frederic Weisbecker
2025-02-06 12:14     ` Oleg Nesterov
2025-02-06 15:57   ` Thomas Gleixner
2025-02-06 16:23     ` Oleg Nesterov [this message]
2025-02-06 21:04       ` Thomas Gleixner
2025-02-05 17:53 ` [PATCH 0/1] " Mateusz Guzik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250206162301.GC5209@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=frederic@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.