Re: [PATCH v3 1/2] rust: add initial scatterlist abstraction

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Aug 04, 2025 at 11:56:53AM +0300, Abdiel Janulgue wrote:
> Hi,
> 
> On 24/07/2025 08:40, Alexandre Courbot wrote:
> > 
> > I see a few issues with the `Item` type here.
> > 
> > The first one is that `Page` can only be created by allocating a new
> > page from scratch using `Page::alloc_page`. This doesn't cover the cases
> > where we want to map memory that is now allocated through this
> > mechanism, e.g. when mapping a `VVec`. So I think we have no choice but
> > return `*mut bindings::page`s.
> > 
> Just commenting on this bit, still going through the others one by one.
> Anyways, there is already existing code I'm working on that should be able
> to extend Page that are not allocated by it's constructor (e.g. those coming
> from vmalloc_to_page). I think's it's safe at least to not expose the raw
> pointers here if we can? Just a thought.

I would try not to expose vmalloc_to_page() to safe rust.

alloc_page() at least gives you a refcounted page with a sensible
refcount based lifecycle, vmalloc_to_page() gives you something that
is not refcountable at all and has a lifetime bound to the vmalloc.

They may both be struct page in C but for rust they have very
different rules and probably types.

If you want kmalloc/vmalloc to get into a scatterlist you should have
APIs to go directly from void * and into the scatterlist, and also
link the scatterlist to the lifetime of the original allocation.

Jason