Message-ID: <0e19e1c3-293b-4740-93f3-2c410893288b@redhat.com>
Date: Tue, 4 Feb 2025 11:26:19 +0100
From: David Hildenbrand <david@...hat.com>
To: Zi Yan <ziy@...dia.com>, Asahi Lina <lina@...hilina.net>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
 Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <benno.lossin@...ton.me>,
 Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>, Jann Horn <jannh@...gle.com>,
 Matthew Wilcox <willy@...radead.org>, Paolo Bonzini <pbonzini@...hat.com>,
 Danilo Krummrich <dakr@...nel.org>, Wedson Almeida Filho
 <wedsonaf@...il.com>, Valentin Obst <kernel@...entinobst.de>,
 Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
 airlied@...hat.com, Abdiel Janulgue <abdiel.janulgue@...il.com>,
 rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
 asahi@...ts.linux.dev, Oscar Salvador <osalvador@...e.de>,
 Muchun Song <muchun.song@...ux.dev>
Subject: Re: [PATCH 0/6] rust: page: Support borrowing `struct page` and
 physaddr conversion

On 03.02.25 22:05, Zi Yan wrote:
> On 3 Feb 2025, at 9:32, Asahi Lina wrote:
> 
>> On 2/3/25 6:58 PM, Simona Vetter wrote:
>>> On Sun, Feb 02, 2025 at 10:05:42PM +0900, Asahi Lina wrote:
>>>> This series refactors the existing Page wrapper to support borrowing
>>>> `struct page` objects without ownership on the Rust side, and converting
>>>> page references to/from physical memory addresses.
>>>>
>>>> The series overlaps with the earlier submission in [1] and follows a
>>>> different approach, based on the discussion that happened there.
>>>>
>>>> The primary use case for this is implementing IOMMU-style page table
>>>> management in Rust. This allows drivers for IOMMUs and MMU-containing
>>>> SoC devices to be written in Rust (such as embedded GPUs). The intended
>>>> logic is similar to how ARM SMMU page tables are managed in the
>>>> drivers/iommu tree.
>>>>
>>>> First, introduce a concept of Owned<T> and an Ownable trait. These are
>>>> similar to ARef<T> and AlwaysRefCounted, but are used for types which
>>>> are not ref counted but rather have a single intended owner.
>>>>
>>>> Then, refactor the existing Page support to use the new mechanism. Pages
>>>> returned from the page allocator are not intended to be ref counted by
>>>> consumers (see previous discussion in [1]), so this keeps Rust's view of
>>>> page ownership as a simple "owned or not". Of course, this is still
>>>> composable as Arc<Owned<Page>> if Rust code needs to reference count its
>>>> own Page allocations for whatever reason.
>>>
>>> I think there's a bit a potential mess here because the conversion to
>>> folios isn't far enough yet that we can entirely ignore page refcounts and
>>> just use folio refcounts. But I guess we can deal with that oddity if we
>>> hit it (maybe folio conversion moves fast enough), since this only really
>>> starts to become relevant for hmm/svm gpu stuff.
>>>
>>> iow I think anticipating the future where struct page really doesn't have
>>> a refcount is the right move. Aside from that it's really not a refcount
>>> that works in the rust ARef sense, since struct page cannot disappear for
>>> system memory, and for dev_pagemap memory it's an entirely different
>>> reference you need (and then there's a few more special cases).
>>
>> Right, as far as this abstraction is concerned, all that needs to hold
>> is that:
>>
>> - alloc_pages() and __free_pages() work as intended, however that may
>> be, to reserve and return one page (for now, though I think extending
>> the Rust abstraction to handle higher-order folios is pretty easy, but
>> that can happen later).
>> - Whatever borrows pages knows what it's doing. In this case there's
>> only support for borrowing pages by physaddr, and it's only going to be
>> used in a driver for a platform without memory hot remove (so far) and
>> only for pages which have known usage (in principle) and are either
>> explicitly allocated or known pinned or reserved, so it's not a problem
>> right now. Future abstractions that return borrowed pages can do their
>> own locking/bookkeeping/whatever is necessary to keep it safe.
>>
>> I would like to hear how memory hot-remove is supposed to work though,
>> to see if we should be doing something to make the abstraction safer
>> (though it's still unsafe and always will be). Is there a chance a
>> `struct page` could vanish out from under us under some conditions?
> 
> Add DavidH and OscarS for memory hot-remove questions.
> 
> IIUC, struct page could be freed if a chunk of memory is hot-removed.

Right, but only after there are no users anymore (IOW, memory was freed 
back to the buddy). PFN walkers might still stumble over them, but I 
would not expect (or recommend) rust to do that.

> 
> Another case struct page can be freed is when hugetlb vmemmap optimization
> is used. Muchun (cc'd) is the maintainer of hugetlbfs.

Here, the "struct page" remains valid though; it can still be accessed, 
although we disallow writes (which would be wrong).

If you only allocate a page and free it later, there is no need to worry 
about either on the rust side.

-- 
Cheers,

David / dhildenb

Powered by blists - more mailing lists