Re: [PATCH 2/2] rust_binder: charge allocated pages to memcg

[Alice Ryhl <aliceryhl@google.com>]

On Fri, Oct 16, 2026 at 11:41:55AM -0700, T.J. Mercier wrote:
> On Fri, Oct 16, 2026 at 6:30 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > Pages allocated for a binder VMA are currently not charged to any memory
> > cgroup. Thus, use `Page::alloc_page_memcg()` to charge them to the
> > owning process's `mm_struct`.
> >
> > Suggested-by: T.J. Mercier <tjmercier@google.com>
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  drivers/android/binder/page_range.rs | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/android/binder/page_range.rs b/drivers/android/binder/page_range.rs
> > index 52ffbf3504e7..57da47243967 100644
> > --- a/drivers/android/binder/page_range.rs
> > +++ b/drivers/android/binder/page_range.rs
> > @@ -27,7 +27,7 @@
> >      ffi::{c_ulong, c_void},
> >      mm::{virt, Mm, MmWithUser},
> >      new_mutex, new_spinlock,
> > -    page::{Page, PAGE_SHIFT, PAGE_SIZE},
> > +    page::{MemcgAllocError, Page, PAGE_SHIFT, PAGE_SIZE},
> >      prelude::*,
> >      str::CStr,
> >      sync::{aref::ARef, Mutex, SpinLock},
> > @@ -399,7 +399,16 @@ pub(crate) fn use_range(&self, start: usize, end: usize) -> Result<()> {
> >      /// Assumes that `i` is in bounds.
> >      #[cold]
> >      unsafe fn use_page_slow(&self, i: usize) -> Result<()> {
> > -        let new_page = Page::alloc_page(GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO)?;
> > +        let new_page = Page::alloc_page_memcg(GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO, &self.mm)
> 
> Not related to this change, but I think it might be better to ditch
> GFP_KERNEL | __GFP_HIGHMEM and use GFP_HIGHUSER. We're already mapping
> this into userspace (and now charging the memcg for it).
> 
> Is current the sender here? If memory.high is exceeded, current gets
> blamed for it in try_charge_memcg()'s done_restock section. So if it's
> the sender, that's not exactly what we want since the sender would be
> throttled for it. So I think this depends on
> https://lore.kernel.org/all/20261010210957.1350874-4-shakeel.butt@linux.dev/.

Yes, current is the sender process here, allocating a page on behalf
of the recipient process (`self.mm`).

Because current is the sender, I don't think we want GFP_HIGHUSER: the
only difference between GFP_KERNEL | __GFP_HIGHMEM and GFP_HIGHUSER is
__GFP_HARDWALL, which enforces current's cpuset (current->mems_allowed)
in cpuset_current_node_allowed(). Enforcing a hardwall against the
sender's cpuset when allocating a page for the recipient doesn't seem
right.

And yes, it looks like we do indeed depend on Shakeel's patch.

Alice