Re: [PATCH RFC 3/4] rust: drm: Add DRM buddy allocator bindings

[Joel Fernandes <joelagnelf@nvidia.com>]

Hi Danilo,

On 10/30/2025 5:27 PM, Danilo Krummrich wrote:
> On Thu Oct 30, 2025 at 8:06 PM CET, Joel Fernandes wrote:
>> +    ///
>> +    /// Returns an [`AllocatedBlocks`] structure that owns the allocated blocks and automatically
>> +    /// frees them when dropped. Allocation of `list_head` uses the `gfp` flags passed.
>> +    pub fn alloc_blocks(
>> +        &self,
>> +        start: usize,
>> +        end: usize,
>> +        size: usize,
>> +        min_block_size: usize,
>> +        flags: BuddyFlags,
>> +        gfp: Flags,
>> +    ) -> Result<AllocatedBlocks<'_>> {
>> +        // Allocate list_head on the heap.
>> +        let mut list_head = KBox::new(bindings::list_head::default(), gfp)?;
>> +
>> +        // SAFETY: list_head is valid and heap-allocated.
>> +        unsafe {
>> +            bindings::INIT_LIST_HEAD(&mut *list_head as *mut _);
>> +        }
> 
> Not a full review, but a quick drive-by comment:
> 
> bindings::list_head has to be pinned in memory it should be
> 
> 	let list_head = KBox::pin_init(Opaque::ffi_init(|slot: *mut bindings::list_head| {
> 	    // SAFETY: `slot` is a valid pointer to uninitialized memory.
> 	    unsafe { bindings::INIT_LIST_HEAD(slot) };
> 	}), gfp)?;
> 

Sure. I will use pin_init here.

> if you're doing it by hand, but as mentioned in a previous patch, I think it
> would be nice to have a transparent wrapper type, CListHead, for this.
I like your CListHead idea. Let me sketch this out a bit more and see what the
lifetime relationships look like, thanks! - Joel