The original design for DmaFence intended to comply as strictly as possible with the "dma_fence contract", which, according to some, says that a fence must only be signaled once. Thus, the implementation was made so that a DriverFence drops on signal.
While extending JobQueue's capabilities, it was discovered that this characteristic causes - unnecessary - trouble: fence-data must be dropped in an RCU-deferred manner because the C backend demands that. However, fences are often signaled in atomic context (e.g., interrupt handler) where you don't want to drop, but signal as quickly as possible. Actually, there is no real hard reason to enforce that a fence can only get signaled once, because the C backend is robust against multiple signal attempts. What is decisive is that a) all fences signal and b) they only drop no earlier than 1 RCU grace period after signaling. Don't drop a DriverFence on signal(). Signed-off-by: Philipp Stanner <[email protected]> --- rust/kernel/dma_buf/dma_fence.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/rust/kernel/dma_buf/dma_fence.rs b/rust/kernel/dma_buf/dma_fence.rs index a1d851656562..3022bdb5f7c3 100644 --- a/rust/kernel/dma_buf/dma_fence.rs +++ b/rust/kernel/dma_buf/dma_fence.rs @@ -805,7 +805,7 @@ pub fn as_fence(&self) -> &Fence { } /// Signal the fence. This will invoke all registered callbacks. - pub fn signal(self, res: Result) { + pub fn signal(&self, res: Result) { let fence = self.as_fence().lock(); // SAFETY: `fence` is valid because `self` is valid. The lock must be @@ -816,14 +816,15 @@ pub fn signal(self, res: Result) { // must not have been signaled yet, which we check directly above. unsafe { bindings::dma_fence_set_error(fence.as_raw(), err.to_errno()) }; } + + // SAFETY: `self.data` is valid because `self` is valid. + let fctx = unsafe { self.data.as_ref().fctx }; + let _ = fctx.nr_of_unsignaled_fences.fetch_sub(1, Relaxed); + // SAFETY: `fence` is valid because `self` is valid. The lock must // be held, which we acquired above. unsafe { bindings::dma_fence_signal_locked(fence.as_raw()) }; } - - // SAFETY: `self.data` is valid because `self` is valid. - let fctx = unsafe { self.data.as_ref().fctx }; - let _ = fctx.nr_of_unsignaled_fences.fetch_sub(1, Relaxed); } } base-commit: 3497a403264b094ebd61b8b755165d56e7d9dc37 -- 2.56.0
