[PATCH] rust: DmaFence: Add better warning through Device reference

[Philipp Stanner]

FenceContext::drop() contains a warning print that warns about possible
memory corruptions if there are forgotten fences. However, precisely
speaking, a forgotten fence is undefined behavior.

Moreover, forgotten fences hint at a severe design problem in the
driver. In this context, the used pr_err!() does not provide very useful
text output.

Replace the warning print with a dev_warn!(). To do so, have the
FenceContext carry a reference to a Device, protected by the already
present lifetime.

Suggested-by: Danilo Krummrich <dakr@xxxxxxxxxx>
Signed-off-by: Philipp Stanner <phasta@xxxxxxxxxx>
---
 rust/kernel/dma_buf/dma_fence.rs | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/dma_buf/dma_fence.rs b/rust/kernel/dma_buf/dma_fence.rs
index 18a43e1bb442..58ba678e91b5 100644
--- a/rust/kernel/dma_buf/dma_fence.rs
+++ b/rust/kernel/dma_buf/dma_fence.rs
@@ -32,6 +32,10 @@
 };
 
 use kernel::{
+    device::{
+        Device,
+        Normal, //
+    },
     str::CString,
     sync::{
         aref::{
@@ -59,7 +63,7 @@ pub trait FenceContextOps {
 /// with each other, providing each with raising sequence numbers and a common
 /// identifier.
 #[pin_data(PinnedDrop)]
-pub struct FenceContext<T: FenceContextOps + Send + Sync> {
+pub struct FenceContext<'a, T: FenceContextOps + Send + Sync> {
     /// The fence context number.
     nr: u64,
     /// The sequence number for the next fence created.
@@ -81,12 +85,14 @@ pub struct FenceContext<T: FenceContextOps + Send + Sync> {
     // the lifetime which intends to enforce that all fences disappear before
     // their context.
     nr_of_unsignaled_fences: Atomic<usize>,
+    /// The device this fence context is associated with.
+    dev: &'a Device<Normal>,
     /// The user's data.
     #[pin]
     data: T,
 }
 
-impl<'a, T: Send + Sync + FenceContextOps> FenceContext<T> {
+impl<'a, T: Send + Sync + FenceContextOps + 'a> FenceContext<'a, T> {
     // This can later be extended as a vtable in case other parties need support
     // for the more "exotic" callbacks.
     const OPS: bindings::dma_fence_ops = bindings::dma_fence_ops {
@@ -106,6 +112,7 @@ pub fn new<E>(
         initial_seqno: u64,
         driver_name: &CStr,
         timeline_name: &CStr,
+        dev: &'a Device<Normal>,
         data: impl PinInit<T, E>,
     ) -> impl PinInit<Self, Error>
     where
@@ -122,6 +129,7 @@ pub fn new<E>(
             driver_name: driver_name?,
             timeline_name: timeline_name?,
             nr_of_unsignaled_fences: Atomic::new(0),
+            dev,
             data <- data,
         })
     }
@@ -211,7 +219,7 @@ unsafe fn from_raw_fence(ptr: *mut bindings::dma_fence) -> &'a Self {
 }
 
 #[pinned_drop]
-impl<T: FenceContextOps + Send + Sync> PinnedDrop for FenceContext<T> {
+impl<T: FenceContextOps + Send + Sync> PinnedDrop for FenceContext<'_, T> {
     fn drop(self: Pin<&mut Self>) {
         // Fence ops callbacks can be called on unsignaled fences. Since these
         // callbacks can access the fence context and its data, it needs to be
@@ -596,7 +604,7 @@ struct DriverFenceData<'a, T: Send + Sync + FenceContextOps> {
     /// Callback head for dropping this in a deferred manner through RCU.
     rcu_head: bindings::callback_head,
     /// Reference to access the FenceContext.
-    fctx: &'a FenceContext<T>,
+    fctx: &'a FenceContext<'a, T>,
     /// The API user's data. It is essential that the data only performs
     /// operations legal in atomic context in its [`Drop`] implementation.
     #[pin]
@@ -641,6 +649,7 @@ struct DriverFenceData<'a, T: Send + Sync + FenceContextOps> {
 ///
 /// ```
 /// use kernel::{
+///     device::Normal,
 ///     dma_buf::{
 ///         DriverFence,
 ///         FenceContext,
@@ -648,6 +657,7 @@ struct DriverFenceData<'a, T: Send + Sync + FenceContextOps> {
 ///         FenceCallback,
 ///         FenceCallbackRegistration,
 ///     },
+///     faux,
 ///     str::CString,
 ///     sync::aref::ARef, //
 /// };
@@ -676,9 +686,10 @@ struct DriverFenceData<'a, T: Send + Sync + FenceContextOps> {
 ///
 /// let fctx_data = FenceContextData::new();
 ///
+/// let reg = faux::Registration::new(c"DmaFence-KUnit-Test", None)?;
 ///
 /// let mut fctx = KBox::pin_init(
-///     FenceContext::new(0, c"dummy_driver", c"dummy_timeline", fctx_data),
+///     FenceContext::new(0, c"dummy_driver", c"dummy_timeline", reg.as_ref().as_ref(), fctx_data),
 ///     GFP_KERNEL
 /// )?;
 ///
@@ -934,7 +945,9 @@ fn drop(&mut self) {
         // SAFETY: `guard` is valid until the `call_rcu()` below.
         let signaled: bool = unsafe { bindings::dma_fence_test_signaled_flag(guard.as_raw()) };
         if !signaled {
-            pr_err!("DriverFence drops unsignaled. Danger of memory corruption!\n");
+            // SAFETY: `data` is valid because `self` is valid.
+            let dev = unsafe { self.data.as_ref().fctx.dev };
+            dev_warn!(dev, "DriverFence drops unsignaled.\n");
             // SAFETY: `guard` is valid until the `call_rcu()` below. The fence
             // must not have been signaled yet, which we check directly above.
             unsafe { bindings::dma_fence_set_error(guard.as_raw(), ECANCELED.to_errno()) };

base-commit: 896ed083362758b33c49a1b5e5a3423c5814d87e
-- 
2.55.0