Re: [PATCH v6 4/9] binder: store shrinker metadata under page->private

[Alice Ryhl]

On Tue, Dec 3, 2024 at 10:56 PM Carlos Llamas <cmllamas@xxxxxxxxxx> wrote:
>
> Instead of pre-allocating an entire array of struct binder_lru_page in
> alloc->pages, install the shrinker metadata under page->private. This
> ensures the memory is allocated and released as needed alongside pages.
>
> By converting the alloc->pages[] into an array of struct page pointers,
> we can access these pages directly and only reference the shrinker
> metadata where it's being used (e.g. inside the shrinker's callback).

Using many allocations instead of a single array will increase the
number of allocations a lot. Is it worth it?

> Rename struct binder_lru_page to struct binder_shrinker_mdata to better
> reflect its purpose. Add convenience functions that wrap the allocation
> and freeing of pages along with their shrinker metadata.
>
> Note I've reworked this patch to avoid using page->lru and page->index
> directly, as Matthew pointed out that these are being removed [1].
>
> Link: https://lore.kernel.org/all/ZzziucEm3np6e7a0@xxxxxxxxxxxxxxxxxxxx/ [1]
> Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> Cc: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> Reviewed-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> Signed-off-by: Carlos Llamas <cmllamas@xxxxxxxxxx>

[...]

> +static void binder_free_page(struct page *page)
> +{
> +       kfree((void *)page_private(page));
> +       __free_page(page);

I would cast the page_private to a pointer of the right type here.
There may be tools or future improvements to kfree that use the type
information.

Alice