Re: [PATCH v2 1/2] rust: introduce abstractions for fwctlg

Danilo Krummrich
2026-01-28T15:49:15Z

On Wed Jan 28, 2026 at 3:59 PM CET, Jason Gunthorpe wrote:
> On Wed, Jan 28, 2026 at 03:01:25PM +0100, Danilo Krummrich wrote:
>> There is no second memory allocation. In the implementation of
>> fwctl::Device::new() above we call _fwctl_alloc_device() with a size (and
>> layout) such that this allocation is suitable to initialize the second argument
>> (i.e. data: impl PinInit<T, Error>) within this allocation.
>
> You are talking about your suggestions now right?

Correct, I'm talking about my proposal.

> Because what I see in Zi's patch doesn't match any of this?
>
> + bindings::_fwctl_alloc_device(
> + parent.as_raw(),
> + ops,
> + core::mem::size_of::<bindings::fwctl_device>(),
> + )
>
> That is not allocating any memory for driver use ...

Yes, the patch doesn't do any of that.

> What you are explaining sounds good to me, though I don't quite get
> the PinInit<> flow but I trust you on that. :)

So, it would basically look like this:

pub struct Device<T> {
dev: Opaque<bindings::fwctl_device>,
data: T,
}

Where T is the type of the driver specific fwctl device data.

impl<T> Device<T> {
pub fn new(
parent: &device::Device,
data: impl PinInit<T::Data, Error>
) -> Result<ARef<Self>> {
let layout = Kmalloc::aligned_layout(Layout::new::<Self>());

// SAFETY: ...
let raw_fwctl: *mut Self = unsafe {
bindings::_fwctl_alloc_device(
parent.as_raw(),
&Self::VTABLE,
layout.size(),
)
}
.cast();
let raw_fwctl = NonNull::new(from_err_ptr(raw_fwctl)?).ok_or(ENOMEM)?;

// Get the pointer to `Self::data`.
let raw_data = unsafe { &raw mut (*raw_fwctl.as_ptr().data) };

// Initialize the `data` initializer within the memory pointed
// to by `raw_data`.
unsafe { data.__pinned_init(raw_data) }.inspect_err(|_| {
// Find the `struct fwctl_device` pointer from the `Self`
// pointer.
let fwctl_dev = unsafe { Self::into_fwcl_device(raw_fwctl) };

// Since we failed to execute the initializer of `data`,
// unwind, i.e. drop our reference to `fwctl_dev`.
unsafe { bindings::fwctl_put(fwctl_dev) };
})?;

// The initializer was successful, hence return `Self` as
// `ARef<Self>`.
Ok(unsafe { ARef::from_raw(raw_fwctl) })
}
}

So, essentially the driver passes an initializer of its private data and we
"write" this initializer into the extra memory allocated with
_fwctl_alloc_device().