On Mon, Mar 11, 2024 at 10:50:56AM +0000, Alice Ryhl wrote: > Alice Ryhl <aliceryhl@xxxxxxxxxx> writes: > > +/// Flags for the "get free page" function that underlies all memory allocations. > > +pub mod flags { > > + pub type gfp_t = bindings::gfp_t; > > + > > + /// `GFP_KERNEL` is typical for kernel-internal allocations. The caller requires `ZONE_NORMAL` > > + /// or a lower zone for direct access but can direct reclaim. > > + pub const GFP_KERNEL: gfp_t = bindings::GFP_KERNEL; > > + /// `GFP_ZERO` returns a zeroed page on success. > > + pub const __GFP_ZERO: gfp_t = bindings::__GFP_ZERO; > > + /// `GFP_HIGHMEM` indicates that the allocated memory may be located in high memory. > > + pub const __GFP_HIGHMEM: gfp_t = bindings::__GFP_HIGHMEM; > > +} > > > > [...] > > > > +impl Page { > > + /// Allocates a new page. > > + pub fn alloc_page(gfp_flags: flags::gfp_t) -> Result<Self, AllocError> { > > + // SAFETY: The specified order is zero and we want one page. > > + let page = unsafe { bindings::alloc_pages(gfp_flags, 0) }; > > + let page = NonNull::new(page).ok_or(AllocError)?; > > + // INVARIANT: We checked that the allocation succeeded. > > + Ok(Self { page }) > > + } > > Matthew Wilcox: You suggested on a previous version that I use gfp flags > here, or that I rename it to e.g. BinderPage to make it clear that this > is specific to the kind of pages that Binder needs. I think what you have here is good. > In this version I added some gfp flags, but I'm not actually sure that > the Page abstraction works for all combinations of gfp flags. For > example, I use kmap_local_page when accessing the page, but is that > correct if there's a user that doesn't pass GFP_HIGHMEM? Yes, kmap_local_page() works for non-highmem pages (it's essentially a no-op)