Re: [PATCH v12 2/3] rust: add dma coherent allocator abstraction.

[Alice Ryhl]

On Mon, Feb 24, 2025 at 12:50 PM Abdiel Janulgue
<abdiel.janulgue@xxxxxxxxx> wrote:
>
> Add a simple dma coherent allocator rust abstraction. Based on
> Andreas Hindborg's dma abstractions from the rnvme driver, which
> was also based on earlier work by Wedson Almeida Filho.
>
> Nacked-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@xxxxxxxxx>

> +    /// Create a duplicate of the `CoherentAllocation` object but prevent it from being dropped.
> +    pub fn skip_drop(self) -> CoherentAllocation<T> {
> +        let me = core::mem::ManuallyDrop::new(self);
> +        Self {
> +            // SAFETY: The refcount of `dev` will not be decremented because this doesn't actually
> +            // duplicafe `ARef` and the use of `ManuallyDrop` forgets the originals.
> +            dev: unsafe { core::ptr::read(&me.dev) },
> +            dma_handle: me.dma_handle,
> +            count: me.count,
> +            cpu_addr: me.cpu_addr,
> +            dma_attrs: me.dma_attrs,
> +        }
> +    }

The skip_drop pattern requires the return value to use a different
struct with the same fields, because otherwise you don't really skip
the destructor. But I don't think you have the user for this method
anymore so maybe just drop it.

> +    /// Retrieve a single entry from the region with bounds checking. `offset` is in units of `T`,
> +    /// not the number of bytes.
> +    pub fn item_from_index(&self, offset: usize) -> Result<*mut T> {
> +        if offset >= self.count {
> +            return Err(EINVAL);
> +        }
> +        // SAFETY:
> +        // - The pointer is valid due to type invariant on `CoherentAllocation`
> +        // and we've just checked that the range and index is within bounds.
> +        // - `offset` can't overflow since it is smaller than `self.count` and we've checked
> +        // that `self.count` won't overflow early in the constructor.
> +        Ok(unsafe { &mut *self.cpu_addr.add(offset) })

The point of the dma_read/dma_write macros is to avoid creating
references to the dma memory, so don't create a reference here.

Ok(unsafe { self.cpu_addr.add(offset) })

Alice