Re: [PATCH v2 1/5] rust: types: add `Owned` type and `Ownable` trait

From: Abdiel Janulgue
Date: Fri Nov 01 2024 - 09:38:31 EST


Hi Alice, Boqun:

On 24/10/2024 10:33, Alice Ryhl wrote:

Please rename this function to from_raw to match the name used by
other similar functions.

Also, I don't love this wording. We don't really want to guarantee
that it is unique. For example, pages have one primary owner, but
there can be others who also have refcounts to the page, so it's not
really unique. I think you just want to say that `ptr` must point at a

But then when `Owned<Page>` dropped, it will call __free_pages() which
invalidate any other existing users. Do you assume that the users will
use pointers anyway, so it's their unsafe responsiblity to guarantee
that they don't use an invalid pointer?

Also I assume you mean the others have refcounts to the page *before* an
`Owned<Page>` is created, right? Because if we really have a use case
where we want to have multiple users of a page after `Owned<Page>`
created, we should better provide a `Owned<Page>` to `ARef<Page>`
function.

The __free_pages function just decrements a refcount. If there are
other references to it, it's not actually freed.


Then why don't we use page_put() there? ;-) And instead of
`Owned<Page>`, we can wrap the kernel::page as `ARef<Page>`, no?

I don't think there's a function called page_put?

Sorry I confused myself. It's because it's called put_page.


How do I proceed with this? Should we use the page's reference count to decide when to free the allocation and use put_page() instead of __free_pages() in Page::Drop?.

In that case, there would be no need for `Ownable`, right? As we could just return ARef<Page> in both vmalloc_to_page() case and in Page::alloc_page(), letting the kernel handle ownership internally.

Regards,
Abdiel