Re: [PATCH v2 2/9] rust: debugfs: add SeqShow trait and seq_file file operations

[Alice Ryhl <aliceryhl@google.com>]

On Fri, Jul 31, 2026 at 01:05:40AM +0800, Alvin Sun wrote:
> +/// Show callback for `SeqShow` types.
> +///
> +/// # Safety
> +///
> +/// The seq_file core guarantees that `seq` points to a live `seq_file` and that
> +/// `seq->private` is a valid pointer to a `T` with no outstanding mutable
> +/// references.
> +unsafe extern "C" fn seq_file_show<S: SeqShow<T>, T: Sync>(
> +    seq: *mut bindings::seq_file,
> +    _: *mut crate::ffi::c_void,
> +) -> crate::ffi::c_int {
> +    // SAFETY: `seq->private` is a valid `T` pointer with no outstanding
> +    // mutable references.
> +    let data = unsafe { &*((*seq).private.cast::<T>()) };
> +    // SAFETY: `seq` points to a live `seq_file`.
> +    let m = unsafe { SeqFile::from_raw(seq) };
> +    from_result(|| S::show(data, m).map(|()| 0))
> +}
> +

> +/// Renders `data` into a seq_file.
> +///
> +/// `data` is the value stashed as the debugfs file's `i_private` at creation
> +/// time. `show` is invoked on each read to produce the file's contents.
> +///
> +/// `open` and `release` are optional lifecycle hooks called during file open
> +/// and release. They can be used to manage the lifetime of `data` (e.g.,
> +/// reference counting). Default implementations are no-ops.
> +pub trait SeqShow<T> {
> +    /// Writes debugfs output for the file.
> +    fn show(data: &T, m: &SeqFile) -> Result;
> +
> +    /// Called during file open, before `single_open`.
> +    fn open(_data: &T) -> Result {
> +        Ok(())
> +    }
> +
> +    /// Called during file release, before `single_release`.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `data` must point to valid memory, kept alive by actions taken in
> +    /// [`Self::open`] (e.g., incrementing a reference count).
> +    unsafe fn release(_data: NonNull<T>) {}
> +}

Hmm. So in a later patch you implement SeqShow in the following manner:

    fn open(dev: &drm::Device<T, drm::Normal>) -> Result {
        // Hold a device reference so the device is not freed while the file is open.
        dev.inc_ref();
        Ok(())
    }

    unsafe fn release(dev: NonNull<drm::Device<T, drm::Normal>>) {
        // Drop the reference taken in `open`.
        // SAFETY: `dev` is valid per this function's safety contract.
        unsafe { drm::Device::<T>::dec_ref(dev) };
    }

I don't understand why this increment/decrement pair is necessary.
Just based on the code, it *looks* like you have them because otherwise
`show()` might get called with a dangling pointer to the drm device, but
the inc_ref() in open() ensures it stays alive.

However, earlier in this patch you said:

> The seq_file core guarantees that `seq` points to a live `seq_file` and
> that `seq->private` is a valid pointer to a `T` with no outstanding
> mutable references.

So based on this, you are saying that open()/release() do not need to do
anything special for the value to stay alive.

These seem contradictory. Could you clarify the situation?

Alice