All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <frederic@kernel.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	linux-kernel@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Waiman Long <longman@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH] task_work: Consume only item at a time while invoking the callbacks.
Date: Wed, 26 Feb 2025 14:08:15 +0100	[thread overview]
Message-ID: <Z78SPz7VaVZpDOYg@localhost.localdomain> (raw)
In-Reply-To: <20250226125048.GC8995@redhat.com>

Le Wed, Feb 26, 2025 at 01:50:48PM +0100, Oleg Nesterov a écrit :
> On 02/21, Sebastian Andrzej Siewior wrote:
> >
> > Yi and syzbot managed to hang the task within task_run().
> >
> > The problem is
> >  task_work_run() -> __fput() -> perf_release() ->
> >  perf_event_release_kernel() -> _free_event() ->
> >  perf_pending_task_sync() -> task_work_cancel() failed ->
> >  rcuwait_wait_event().
> >
> > Once task_work_run() is running, the list of callbacks removed from the
> > task_struct and from this point on task_work_cancel() can't remove any
> > pending and not yet started work items.
> 
> But can this patch really solve the problem?
> 
> Suppose we have two tasks, T1 and T2.
> 
> T1 does fd = perf_event_open(pid => T2->pid);
> T2 does fd = perf_event_open(pid => T1->pid);
> 
> Now, both T1 and T2 do close(fd), call task_work_run(), dequeue the
> ____fput work, and finally call __fput(). Now suppose that both perf
> events fire before T1/T2 call perf_event_release_kernel/_free_event.
> 
> Now, T1 and T2 will hang forever in perf_pending_task_sync() waiting
> for each other. task_work_cancel(current) can't succeed with or without
> this patch.
> 
> No?

Duh!

So indeed, the wait/wake based solution is too fragile. Are we back to the
old serialized workqueue days flavour of deadlocks with task work?

Anyway the perf_pending_task()'s put_event() based solution thing should fix
that scenario too.

Thanks.

      reply	other threads:[~2025-02-26 13:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-21 17:05 [PATCH] task_work: Consume only item at a time while invoking the callbacks Sebastian Andrzej Siewior
2025-02-23 22:40 ` Oleg Nesterov
2025-02-25 14:28   ` Frederic Weisbecker
2025-02-25 16:35     ` Oleg Nesterov
2025-02-25 22:20       ` Frederic Weisbecker
2025-02-26 13:13         ` Oleg Nesterov
2025-02-26 14:01           ` Oleg Nesterov
2025-02-26 14:42             ` Frederic Weisbecker
2025-02-26 18:36               ` Oleg Nesterov
2025-02-26 14:16   ` Sebastian Andrzej Siewior
2025-02-26 14:29     ` Oleg Nesterov
2025-02-26 14:32       ` Sebastian Andrzej Siewior
2025-02-26 12:50 ` Oleg Nesterov
2025-02-26 13:08   ` Frederic Weisbecker [this message]

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=Z78SPz7VaVZpDOYg@localhost.localdomain \
    --to=frederic@kernel.org \
    --cc=bigeasy@linutronix.de \
    --cc=edumazet@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=oleg@redhat.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.