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

Joel Fernandes
2025-10-30T22:49:19Z

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