All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Jason Gunthorpe" <jgg@ziepe.ca>
Cc: "Abdiel Janulgue" <abdiel.janulgue@gmail.com>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	lyude@redhat.com, "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Tamir Duberstein" <tamird@gmail.com>,
	"FUJITA Tomonori" <fujita.tomonori@gmail.com>,
	"open list" <linux-kernel@vger.kernel.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Randy Dunlap" <rdunlap@infradead.org>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"Caleb Sander Mateos" <csander@purestorage.com>,
	"Petr Tesarik" <petr@tesarici.cz>,
	"Sui Jingfeng" <sui.jingfeng@linux.dev>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Robin Murphy" <robin.murphy@arm.com>,
	airlied@redhat.com,
	"open list:DMA MAPPING HELPERS" <iommu@lists.linux.dev>,
	rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v3 1/2] rust: add initial scatterlist abstraction
Date: Tue, 05 Aug 2025 18:12:44 +0200	[thread overview]
Message-ID: <DBUMKISHUL6D.UVSF04ZRQR9Z@kernel.org> (raw)
In-Reply-To: <20250805154222.GS26511@ziepe.ca>

On Tue Aug 5, 2025 at 5:42 PM CEST, Jason Gunthorpe wrote:
> 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.

Agreed, not directly at least, more below.

> 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.

For now they actually have, i.e. BorrowedPage<'a> [1], but this will go away
once we have the Ownable trait and Owned type. Once we have that we can
represent a borrowed page as &'a Page. Where 'a represents the lifetime of the
reference in both cases.

Let me sketch up how the lifetime of a page is modeled if the page is owned by
some other entity, let's say a vmalloc allocation through VBox.

First we have a trait which represents the owner of a Page that we can borrow
the page from:

	pub trait PageOwner {
	    fn borrow_page_at<'a>(&'a mut self, n: usize) -> Result<BorrowedPage<'a>>;
	}

The Vmalloc allocator can provide a helper for vmalloc_to_page(), but this is
not an API that should be used by drivers directly:

	impl Vmalloc {
	    pub unsafe fn to_page<'a>(ptr: NonNull<u8>) -> page::BorrowedPage<'a> {
	        // SAFETY: `ptr` is a valid pointer to `Vmalloc` memory.
	        let page = unsafe { bindings::vmalloc_to_page(ptr.as_ptr().cast()) };
	
	        // SAFETY: `vmalloc_to_page` returns a valid pointer to a `struct page` for a valid pointer
	        // to `Vmalloc` memory.
	        let page = unsafe { NonNull::new_unchecked(page) };
	
	        // SAFETY:
	        // - `self.0` is a valid pointer to a `struct page`.
	        // - `self.0` is valid for the entire lifetime of `'a`.
	        unsafe { page::BorrowedPage::from_raw(page) }
	    }
	}

The implementation of VBox could look like this:

	impl<T> PageOwner for VBox<T> {
	    fn borrow_page_at<'a>(&'a mut self, n: usize) -> Result<BorrowedPage<'a>> {
	        // Calculate offset of the Nth page of the VBox and store it in `ptr`.
	
	        unsafe { Vmalloc::to_page(ptr) }
	    }
	}

(Actually, we may want to use some iterator instead. I'm not sure yet, but
either way, the same principle does apply.)

Finally, if you have some VBox you can borrow a Page list this:

	let mut vbox = VBox::<[u8; PAGE_SIZE]>::new_uninit(GFP_KERNEL)?;

	// Get the first page of the `vbox`.
	let page = borrow_page_at(&mut vbox, 0)?;

Note that the lifetime of page is now bound to the lifetime of vbox.

Analogous, any entity that owns one or multiple pages can implement the
PageOwner trait in a similar way.

For the scatterlist abstractions, we're mostly interested in VVec for now.

For an owned SGTable we would consume a value of some generic type P that
implements PageOwner (P: PageOwner), or whatever we call it in the end.

> 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.

[1] https://lore.kernel.org/rust-for-linux/20250804195023.150399-1-dakr@kernel.org/

  reply	other threads:[~2025-08-05 16:12 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-18 10:33 [PATCH v3 0/2] rust: add initial scatterlist abstraction Abdiel Janulgue
2025-07-18 10:33 ` [PATCH v3 1/2] " Abdiel Janulgue
2025-07-23  0:44   ` Daniel Almeida
2025-07-24  5:40   ` Alexandre Courbot
2025-08-04  8:56     ` Abdiel Janulgue
2025-08-05 15:42       ` Jason Gunthorpe
2025-08-05 16:12         ` Danilo Krummrich [this message]
2025-07-24 20:19   ` Lyude Paul
2025-07-26 14:10     ` Alexandre Courbot
2025-08-01 18:26   ` Robin Murphy
2025-08-04  9:04     ` Alexandre Courbot
2025-08-08 13:13   ` Andreas Hindborg
2025-08-08 20:23     ` Benno Lossin
2025-07-18 10:33 ` [PATCH v3 2/2] samples: rust: add sample code for " Abdiel Janulgue
2025-07-23  0:54   ` Daniel Almeida
2025-07-23  8:07     ` Alexandre Courbot
2025-08-08 12:18   ` Andreas Hindborg
2025-07-18 10:50 ` [PATCH v3 0/2] rust: add initial " Danilo Krummrich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DBUMKISHUL6D.UVSF04ZRQR9Z@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=abdiel.janulgue@gmail.com \
    --cc=acourbot@nvidia.com \
    --cc=airlied@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=csander@purestorage.com \
    --cc=fujita.tomonori@gmail.com \
    --cc=gary@garyguo.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=iommu@lists.linux.dev \
    --cc=jgg@ziepe.ca \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=m.szyprowski@samsung.com \
    --cc=ojeda@kernel.org \
    --cc=petr@tesarici.cz \
    --cc=rdunlap@infradead.org \
    --cc=robin.murphy@arm.com \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sui.jingfeng@linux.dev \
    --cc=tamird@gmail.com \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.