From: sashiko-bot@kernel.org
To: "Philipp Stanner" <phasta@kernel.org>
Cc: media-ci@linuxtv.org, dri-devel@lists.freedesktop.org, ojeda@kernel.org
Subject: Re: [PATCH v9 4/5] rust: Add dma_fence abstractions
Date: Wed, 05 Aug 2026 15:46:25 +0000 [thread overview]
Message-ID: <20260805154626.5A1761F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260805145949.938505-6-phasta@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Calling Fence::is_signaled() from within a fence callback for the same fence will cause a deadlock.
- [High] FenceContext::drop unconditionally calls rcu_barrier(), which can lead to sleeping in atomic context panics and severe performance issues.
--
--- Patch [4]: [PATCH v9 4/5] rust: Add dma_fence abstractions ---
commit 38c478417a422309c2a228acac8f860a0f96dd34
Author: Philipp Stanner <phasta@kernel.org>
rust: Add dma_fence abstractions
This commit introduces abstractions for C's dma_fence to be used by Rust GPU
drivers. It implements DriverFence and Fence types to handle signaling,
sequence numbering, and callback registration.
> diff --git a/rust/kernel/dma_buf/dma_fence.rs b/rust/kernel/dma_buf/dma_fence.rs
> new file mode 100644
> index 0000000000000..e61b4b2d8b8cc
> --- /dev/null
> +++ b/rust/kernel/dma_buf/dma_fence.rs
[ ... ]
> +#[pinned_drop]
> +impl<T: FenceContextOps + Send + Sync> PinnedDrop for FenceContext<T> {
> + fn drop(self: Pin<&mut Self>) {
> + // TODO:
> + // It would be better if the fence context signals all forgotten fences
> + // itself. To do so, it would keep a list of unsignaled fences. That
> + // list members would have to be pre-allocated (see
> + // FenceCallback::new_fence_allocation()).
> + if self.nr_of_unsignaled_fences.load(Relaxed) != 0 {
> + panic!("Forgotten fences in FenceContext.");
> + }
> +
> + rcu_barrier();
[Severity: High]
Can dropping a FenceContext in atomic context cause a panic here?
Because Rust drops objects implicitly when they go out of scope, a
FenceContext dropped in an atomic context (such as inside an interrupt
handler or under a spinlock) will trigger a 'sleeping in atomic context'
panic since rcu_barrier() blocks system-wide.
Even if dropped safely in process context, could waiting on a system-wide
RCU barrier during the destruction of every FenceContext cause severe
performance bottlenecks?
[ ... ]
> + pub fn is_signaled(&self) -> bool {
[ ... ]
> + // The fundamental reason is that the C backend currently does guard
> + // setting of the fence's signaled-bit with the fence's spinlock, but
> + // reading is done locklessly.
> + //
> + // See commit c8a5d5ea3ba6a.
> +
> + let _ = self.lock();
> +
> + ret
> + }
> +
> + fn lock(&self) -> FenceGuard {
> + let mut guard = FenceGuard {
> + inner: self.as_raw(),
> + flags: 0,
> + };
> +
> + // SAFETY: fence is valid because self is valid. flag_ptr is
> + // merely a pointer to an integer, whose lifetime is tied to the guard
> + // object.
> + unsafe { bindings::dma_fence_lock_irqsave(guard.inner, &raw mut guard.flags) };
[Severity: High]
Could this result in a deadlock if Fence::is_signaled() is called from within
a fence callback?
When a fence is signaled, the C backend (dma_fence_signal_locked) acquires
the fence's spinlock and iterates over the registered callbacks, executing
them synchronously while holding the lock.
If a Rust FenceCallback implementation calls is_signaled() on that same fence,
it invokes self.lock() which will attempt to acquire the exact same spinlock
via dma_fence_lock_irqsave(), leading to a non-recursive spinlock deadlock.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805145949.938505-2-phasta@kernel.org?part=4
next prev parent reply other threads:[~2026-08-05 15:46 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-05 14:59 [PATCH v9 0/5] rust / dma_buf: Add abstractions for dma_fence Philipp Stanner
2026-08-05 14:59 ` [PATCH v9 1/5] rust: error: add remaining error codes Philipp Stanner
2026-08-05 14:59 ` [PATCH v9 2/5] rust: types: implement ForeignOwnable for ARef<T> Philipp Stanner
2026-08-05 15:22 ` sashiko-bot
2026-08-05 18:23 ` Gary Guo
2026-08-06 14:31 ` Gary Guo
2026-08-05 14:59 ` [PATCH v9 3/5] rust: sync: Add abstraction for rcu_barrier() Philipp Stanner
2026-08-05 19:08 ` Gary Guo
2026-08-05 14:59 ` [PATCH v9 4/5] rust: Add dma_fence abstractions Philipp Stanner
2026-08-05 15:35 ` Daniel Almeida
2026-08-05 15:46 ` sashiko-bot [this message]
2026-08-06 13:56 ` Gary Guo
2026-08-11 9:46 ` Philipp Stanner
2026-08-05 14:59 ` [PATCH v9 5/5] MAINTAINERS: Add entry for Rust dma-buf Philipp Stanner
2026-08-05 16:11 ` [PATCH v9 0/5] rust / dma_buf: Add abstractions for dma_fence Danilo Krummrich
2026-08-10 6:00 ` Miguel Ojeda
2026-08-10 7:47 ` Philipp Stanner
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=20260805154626.5A1761F00A3D@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=media-ci@linuxtv.org \
--cc=ojeda@kernel.org \
--cc=phasta@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/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.