| Message ID | 149ff5a762997c723880751e8a4019907a0b6457.1720534425.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fix task_work interation with freezing | expand |
On 07/09, Pavel Begunkov wrote: > > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -2600,6 +2600,14 @@ static void do_freezer_trap(void) > spin_unlock_irq(¤t->sighand->siglock); > cgroup_enter_frozen(); > schedule(); > + > + /* > + * We could've been woken by task_work, run it to clear > + * TIF_NOTIFY_SIGNAL. The caller will retry if necessary. > + */ > + clear_notify_signal(); > + if (unlikely(task_work_pending(current))) > + task_work_run(); > } Acked-by: Oleg Nesterov <oleg@redhat.com>
On Tue, Jul 09, 2024 at 03:27:19PM +0100, Pavel Begunkov wrote: > io_uring can asynchronously add a task_work while the task is getting > freezed. TIF_NOTIFY_SIGNAL will prevent the task from sleeping in > do_freezer_trap(), and since the get_signal()'s relock loop doesn't > retry task_work, the task will spin there not being able to sleep > until the freezing is cancelled / the task is killed / etc. > > Cc: stable@vger.kernel.org > Link: https://github.com/systemd/systemd/issues/33626 > Fixes: 12db8b690010c ("entry: Add support for TIF_NOTIFY_SIGNAL") > Reported-by: Julian Orth <ju.orth@gmail.com> > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> I haven't looked at the signal code for too long to be all that useful but the problem described and the patch does make sense to me. FWIW, Acked-by: Tejun Heo <tj@kernel.org> Maybe note that this is structured specifically to ease backport and we need further cleanups? It's not great that this is special cased in do_freezer_trap() instead of being integrated into the outer loop. Thanks.
On 7/10/24 01:57, Tejun Heo wrote: > On Tue, Jul 09, 2024 at 03:27:19PM +0100, Pavel Begunkov wrote: >> io_uring can asynchronously add a task_work while the task is getting >> freezed. TIF_NOTIFY_SIGNAL will prevent the task from sleeping in >> do_freezer_trap(), and since the get_signal()'s relock loop doesn't >> retry task_work, the task will spin there not being able to sleep >> until the freezing is cancelled / the task is killed / etc. >> >> Cc: stable@vger.kernel.org >> Link: https://github.com/systemd/systemd/issues/33626 >> Fixes: 12db8b690010c ("entry: Add support for TIF_NOTIFY_SIGNAL") >> Reported-by: Julian Orth <ju.orth@gmail.com> >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > > I haven't looked at the signal code for too long to be all that useful but > the problem described and the patch does make sense to me. FWIW, > > Acked-by: Tejun Heo <tj@kernel.org> > > Maybe note that this is structured specifically to ease backport and we need > further cleanups? It's not great that this is special cased in I'll add a couple of words > do_freezer_trap() instead of being integrated into the outer loop. v1 had it in the common loop, but might actually be good it's limited to freezing, need more digging.
diff --git a/kernel/signal.c b/kernel/signal.c index 1f9dd41c04be..60c737e423a1 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2600,6 +2600,14 @@ static void do_freezer_trap(void) spin_unlock_irq(¤t->sighand->siglock); cgroup_enter_frozen(); schedule(); + + /* + * We could've been woken by task_work, run it to clear + * TIF_NOTIFY_SIGNAL. The caller will retry if necessary. + */ + clear_notify_signal(); + if (unlikely(task_work_pending(current))) + task_work_run(); } static int ptrace_signal(int signr, kernel_siginfo_t *info, enum pid_type type)
io_uring can asynchronously add a task_work while the task is getting freezed. TIF_NOTIFY_SIGNAL will prevent the task from sleeping in do_freezer_trap(), and since the get_signal()'s relock loop doesn't retry task_work, the task will spin there not being able to sleep until the freezing is cancelled / the task is killed / etc. Cc: stable@vger.kernel.org Link: https://github.com/systemd/systemd/issues/33626 Fixes: 12db8b690010c ("entry: Add support for TIF_NOTIFY_SIGNAL") Reported-by: Julian Orth <ju.orth@gmail.com> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- kernel/signal.c | 8 ++++++++ 1 file changed, 8 insertions(+)