mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Philipp Stanner <phasta@mailbox.org>
To: "Christian König" <christian.koenig@amd.com>,
	"Alice Ryhl" <aliceryhl@google.com>
Cc: Boris Brezillon <boris.brezillon@collabora.com>,
	phasta@kernel.org,  Danilo Krummrich <dakr@kernel.org>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	Gary Guo <gary@garyguo.net>, Benno Lossin <lossin@kernel.org>,
	 Daniel Almeida <daniel.almeida@collabora.com>,
	Joel Fernandes <joelagnelf@nvidia.com>,
	 linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	 rust-for-linux@vger.kernel.org
Subject: Re: [RFC PATCH 2/4] rust: sync: Add dma_fence abstractions
Date: Tue, 17 Feb 2026 15:03:57 +0100	[thread overview]
Message-ID: <3d90656315ab0b52f4725ca7c2cd10859d1e4f69.camel@mailbox.org> (raw)
In-Reply-To: <d9e00d8c-93c0-45f8-95f2-3235fb781e7a@amd.com>

On Tue, 2026-02-10 at 16:45 +0100, Christian König wrote:
> On 2/10/26 16:07, Alice Ryhl wrote:
> > On Tue, Feb 10, 2026 at 02:56:52PM +0100, Christian König wrote:
> > > On 2/10/26 14:49, Alice Ryhl wrote:
> > > > On Tue, Feb 10, 2026 at 02:26:31PM +0100, Boris Brezillon wrote:
> > > > > On Tue, 10 Feb 2026 13:15:31 +0000
> > > > > Alice Ryhl <aliceryhl@google.com> wrote:
> > > > > 
> > > > > > On Tue, Feb 10, 2026 at 01:36:17PM +0100, Boris Brezillon wrote:
> > > > > > > On Tue, 10 Feb 2026 10:15:04 +0000
> > > > > > > Alice Ryhl <aliceryhl@google.com> wrote:
> > > > > > >   
> > > > > > > > impl MustBeSignalled<'_> {
> > > > > > > >     /// Drivers generally should not use this one.
> > > > > > > >     fn i_promise_it_will_be_signalled(self) -> WillBeSignalled { ... }
> > > > > > > > 
> > > > > > > >     /// One way to ensure the fence has been signalled is to signal it.
> > > > > > > >     fn signal_fence(self) -> WillBeSignalled {
> > > > > > > >         self.fence.signal();
> > > > > > > >         self.i_promise_it_will_be_signalled()
> > > > > > > >     }
> > > > > > > > 
> > > > > > > >     /// Another way to ensure the fence will be signalled is to spawn a
> > > > > > > >     /// workqueue item that promises to signal it.
> > > > > > > >     fn transfer_to_wq(
> > > > > > > >         self,
> > > > > > > >         wq: &Workqueue,
> > > > > > > >         item: impl DmaFenceWorkItem,
> > > > > > > >     ) -> WillBeSignalled {
> > > > > > > >         // briefly obtain the lock class of the wq to indicate to
> > > > > > > >         // lockdep that the signalling path "blocks" on arbitrary jobs
> > > > > > > >         // from this wq completing
> > > > > > > >         bindings::lock_acquire(&wq->key);
> > > > > > > >         bindings::lock_release(&wq->key);
> > > > > > > > 
> > > > > > > >         // enqueue the job
> > > > > > > >         wq.enqueue(item, wq);
> > > > > > > > 
> > > > > > > >         // The signature of DmaFenceWorkItem::run() promises to arrange
> > > > > > > >         // for it to be signalled.
> > > > > > > >         self.i_promise_it_will_be_signalled()
> > > > > > > >     }  
> > > > > > > 
> > > > > > > I guess what's still missing is some sort of `transfer_to_hw()`
> > > > > > > function and way to flag the IRQ handler taking over the fence
> > > > > > > signaling token.  
> > > > > > 
> > > > > > Yes, transfer to hardware needs to be another piece of logic similar to
> > > > > > transfer to wq. And I imagine there are many ways such a transfer to
> > > > > > hardware could work.
> > > > > > 
> > > > > > Unless you have a timeout on it, in which case the WillBeSignalled is
> > > > > > satisfied by the fact you have a timeout alone, and the signalling that
> > > > > > happens from the irq is just an opportunistic signal from outside the
> > > > > > dma fence signalling critical path.
> > > > > 
> > > > > Yes and no. If it deadlocks in the completion WorkItem because of
> > > > > allocations (or any of the forbidden use cases), I think we want to
> > > > > catch that, because that's a sign fences are likely to end up with
> > > > > timeouts when they should have otherwise been signaled properly.
> > > > > 
> > > > > > Well ... unless triggering timeouts can block on GFP_KERNEL
> > > > > > allocations...
> > > > > 
> > > > > I mean, the timeout handler should also be considered a DMA-signalling
> > > > > path, and the same rules should apply to it.
> > > > 
> > > > I guess that's fair. Even with a timeout you want both to be signalling
> > > > path.
> > > > 
> > > > I guess more generally, if a fence is signalled by mechanism A or B,
> > > > whichever happens first, you have the choice between:
> > > 
> > > That doesn't happen in practice.
> > > 
> > > For each fence you only have one signaling path you need to guarantee
> > > forward progress for.
> > > 
> > > All other signaling paths are just opportunistically optimizations
> > > which *can* signal the fence, but there is no guarantee that they
> > > will.
> > > 
> > > We used to have some exceptions to that, especially around aborting
> > > submissions, but those turned out to be a really bad idea as well.  
> > > 
> > > Thinking more about it you should probably enforce that there is only
> > > one signaling path for each fence signaling.
> > 
> > I'm not really convinced by this.
> > 
> > First, the timeout path must be a fence signalling path because the
> > reason you have a timeout in the first place is because the hw might
> > never signal the fence. So if the timeout path deadlocks on a
> > kmalloc(GFP_KERNEL) and the hw never comes around to wake you up, boom.
> 
> Mhm, good point. On the other hand the timeout handling should probably be considered part of the normal signaling path.


Why would anyone want to allocate in a timeout path in the first place – especially for jobqueue?

Timeout -> close the associated ring. Done.
JobQueue will signal the done_fences with -ECANCELED. 

What would the driver want to allocate in its timeout path, i.e.: timeout callback.


P.

  parent reply	other threads:[~2026-02-17 14:04 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-03  8:13 [RFC PATCH 0/4] Add dma_fence abstractions and DRM Jobqueue Philipp Stanner
2026-02-03  8:14 ` [RFC PATCH 1/4] rust: list: Add unsafe for container_of Philipp Stanner
2026-02-03 15:25   ` Gary Guo
2026-02-04 10:30   ` Alice Ryhl
2026-02-03  8:14 ` [RFC PATCH 2/4] rust: sync: Add dma_fence abstractions Philipp Stanner
2026-02-05  8:57   ` Boris Brezillon
2026-02-06 10:23     ` Danilo Krummrich
2026-02-09  8:19       ` Philipp Stanner
2026-02-09 14:58         ` Boris Brezillon
2026-02-10  8:16           ` Christian König
2026-02-10  8:38             ` Alice Ryhl
2026-02-10  9:06               ` Philipp Stanner
2026-02-10  9:54                 ` Christian König
2026-02-10  9:15               ` Boris Brezillon
2026-02-10 10:15                 ` Alice Ryhl
2026-02-10 10:36                   ` Danilo Krummrich
2026-02-10 10:46                     ` Christian König
2026-02-10 11:40                       ` Alice Ryhl
2026-02-10 12:28                         ` Boris Brezillon
2026-02-11  9:57                         ` Danilo Krummrich
2026-02-11 10:08                           ` Philipp Stanner
2026-02-11 10:28                             ` Boris Brezillon
2026-02-11 10:20                           ` Boris Brezillon
2026-02-11 11:00                             ` Danilo Krummrich
2026-02-11 11:12                               ` Boris Brezillon
2026-02-11 14:38                                 ` Danilo Krummrich
2026-02-11 15:00                                   ` Boris Brezillon
2026-02-11 15:05                                     ` Danilo Krummrich
2026-02-11 15:14                                       ` Boris Brezillon
2026-02-11 15:16                                         ` Danilo Krummrich
2026-03-13 17:27                                     ` Matthew Brost
2026-02-10 10:46                   ` Boris Brezillon
2026-02-10 11:34                   ` Boris Brezillon
2026-02-10 11:45                     ` Alice Ryhl
2026-02-10 12:21                       ` Boris Brezillon
2026-02-10 13:34                         ` Alice Ryhl
2026-02-10 12:36                   ` Boris Brezillon
2026-02-10 13:15                     ` Alice Ryhl
2026-02-10 13:26                       ` Boris Brezillon
2026-02-10 13:49                         ` Alice Ryhl
2026-02-10 13:56                           ` Christian König
2026-02-10 14:00                             ` Philipp Stanner
2026-02-10 14:06                               ` Christian König
2026-02-10 15:32                                 ` Philipp Stanner
2026-02-10 15:50                                   ` Christian König
2026-02-10 15:07                             ` Alice Ryhl
2026-02-10 15:45                               ` Christian König
2026-02-11  8:16                                 ` Philipp Stanner
2026-02-17 14:03                                 ` Philipp Stanner [this message]
2026-02-17 14:09                                   ` Alice Ryhl
2026-02-17 14:22                                     ` Christian König
2026-02-17 14:28                                       ` Philipp Stanner
2026-02-17 14:44                                         ` Danilo Krummrich
2026-03-13 23:20                                           ` Matthew Brost
2026-02-17 15:01                                         ` Christian König
2026-02-18  9:50                                         ` Alice Ryhl
2026-02-18 10:48                                           ` Boris Brezillon
2026-02-10 12:49                   ` Boris Brezillon
2026-02-10 12:56                     ` Boris Brezillon
2026-02-10 13:26                     ` Alice Ryhl
2026-02-10 13:51                       ` Boris Brezillon
2026-02-10 14:11                         ` Alice Ryhl
2026-02-10 14:50                           ` Boris Brezillon
2026-02-11  8:16                             ` Alice Ryhl
2026-02-11  9:20                               ` Boris Brezillon
2026-02-10  9:26               ` Christian König
2026-02-05 10:16   ` Boris Brezillon
2026-02-05 13:16     ` Gary Guo
2026-02-06  9:32       ` Philipp Stanner
2026-02-06 10:16         ` Danilo Krummrich
2026-02-06 13:24           ` Philipp Stanner
2026-02-06 11:04         ` Boris Brezillon
2026-02-09  8:21           ` Philipp Stanner
2026-02-06 11:23         ` Boris Brezillon
2026-02-09 11:30   ` Alice Ryhl
2026-02-03  8:14 ` [RFC PATCH 3/4] rust/drm: Add DRM Jobqueue Philipp Stanner
2026-02-10 14:57   ` Boris Brezillon
2026-02-11 10:47     ` Philipp Stanner
2026-02-11 11:07       ` Boris Brezillon
2026-02-11 11:19         ` Danilo Krummrich
2026-02-11 12:10           ` Boris Brezillon
2026-02-11 12:32             ` Danilo Krummrich
2026-02-11 12:51               ` Boris Brezillon
2026-02-11 11:19         ` Philipp Stanner
2026-02-11 11:59           ` Boris Brezillon
2026-02-11 12:14             ` Philipp Stanner
2026-02-11 12:24               ` Boris Brezillon
2026-02-11 12:22           ` Alice Ryhl
2026-02-11 12:44             ` Philipp Stanner
2026-02-11 12:52               ` Alice Ryhl
2026-02-11 13:53                 ` Philipp Stanner
2026-02-11 15:28                   ` Alice Ryhl
2026-02-11 12:45             ` Danilo Krummrich
2026-02-11 13:45             ` Gary Guo
2026-02-11 14:07               ` Boris Brezillon
2026-02-11 15:17                 ` Alice Ryhl
2026-02-11 15:20                   ` Philipp Stanner
2026-02-11 15:51                     ` Boris Brezillon
2026-02-11 15:53                     ` Alice Ryhl
2026-02-11 15:54                     ` Danilo Krummrich
2026-02-11 15:33               ` Alice Ryhl
2026-02-03  8:14 ` [RFC PATCH 4/4] samples: rust: Add jobqueue tester Philipp Stanner
2026-02-03 16:46 ` [RFC PATCH 0/4] Add dma_fence abstractions and DRM Jobqueue Daniel Almeida

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=3d90656315ab0b52f4725ca7c2cd10859d1e4f69.camel@mailbox.org \
    --to=phasta@mailbox.org \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=boris.brezillon@collabora.com \
    --cc=christian.koenig@amd.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=phasta@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®