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

[Boqun Feng <boqun.feng@gmail.com>]

On Thu, Jun 05, 2025 at 01:02:04PM -0300, Jason Gunthorpe wrote:
> On Thu, Jun 05, 2025 at 08:35:59AM -0700, Boqun Feng wrote:
> > Late to the party, but seems to me the main problem here is that we
> > cannot pass a reference to .set_page(), note that there is some work
> > that would change the Rust struct Page from a `*mut page` to a
> > `page`[0], and then we can impl Ownable[1] and AlwaysRefCounted for
> > `Page`, if that's done, then I believe the correct parameter for
> > set_page() would be an ARef<Page>.
> 
> There are alot of things that want to go into scatterlists that don't
> have struct pages that are refcountable (eg frozen pages used by
> kmalloc).
> 

I see.

> So I don't think you want to go in the direction of forcing
> struct page refcounting in scatter table. That is not how the C API
> works for good reason.
> 

Ok, that's fair.

> I also don't think it is a good idea to push more struct page stuff
> into Rust as we are trying to eliminate struct page from the

Well, that's why I suggested Owned<Page> and ARef<Folio>, but let's not
get too deep into that direction unless necessary ;-)

> kernel. It is better for rust to stick to KVAs and convert to struct
> page in core code only where absolutely necessary for someon reason.
> 

Sounds reasonable to me for now.

Regards,
Boqun

> Which is another part of why I suggested set_page should not be part
> of the driver facing rust API for scatterlist.
> 
> Jason