Re: [PATCH v2 27/28] binder: reverse locking order in shrinker callback

From: Alice Ryhl
Date: Mon Dec 04 2023 - 09:47:38 EST


On Mon, Dec 4, 2023 at 3:45 PM Carlos Llamas <cmllamas@xxxxxxxxxx> wrote:
>
> On Mon, Dec 04, 2023 at 11:57:42AM +0000, 'Alice Ryhl' via kernel-team wrote:
> > > + trace_binder_unmap_kernel_start(alloc, index);
> > > +
> > > + page_to_free = page->page_ptr;
> > > + page->page_ptr = NULL;
> > > +
> > > + trace_binder_unmap_kernel_end(alloc, index);
> >
> > What are these trace calls used for? Previously they wrapped the call to
> > __free_page, and happened after the `trace_binder_unmap_user_*` calls.
> >
>
> It also used to wrap an unmap_kernel_range() call, which explains the
> naming of the tracepoint I suppose. It looked like this:
>
> trace_binder_unmap_kernel_start(alloc, index);
>
> unmap_kernel_range(page_addr, PAGE_SIZE);
> __free_page(page->page_ptr);
> page->page_ptr = NULL;
>
> trace_binder_unmap_kernel_end(alloc, index);
>
> My thinking was that we care more about the page->page_ptr clearing but
> it sounds like we could just drop the tracepoint, we no longer have a
> kernel mapped area.

Ok. Feel free to remove or move the trace calls however it needs to be
done and add my tag:

Reviewed-by: Alice Ryhl <aliceryhl@xxxxxxxxxx>