On 7/31/26 21:08, Alice Ryhl wrote:
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?
You're right. The `SeqShow` trait documentation was inaccurate -- the
framework does not manage the lifetime of `data`, so `inc_ref`/`dec_ref`
in `DrmSeqShow` is necessary to prevent `show()` from accessing a freed
device.
Will fix the doc in the next revision.
Best regards,
Alvin
Alice
