Re: [PATCH] mm: free idle swap cache page after COW

[Linus Torvalds]

On Tue, Jun 1, 2021 at 5:00 AM Johannes Weiner <hannes@xxxxxxxxxxx> wrote:
>
> It's free_page[s]_and_swap_cache() we should reconsider, IMO.
>
> free_swap_cache() makes for a clean API function that does one thing,
> and does it right. free_page_and_swap_cache() combines two independent
> operations, which has the habit of accumulating special case-handling
> for some callers that is unncessary overhead for others (Abstraction
> Inversion anti-pattern).

Agreed. That "free_page_and_swap_cache()" function is odd. Much better
written as

        free_swap_cache();
        free_page();

because there's no real advantage to try to merge the two.

It's not like the merged function can actually do any optimization
based on the merging, it just does the above anyway - except it then
has that extra odd huge_zero_page test that makes no sense
what-so-ever.

So if anything we should try to get rid of uses of that odd
free_page_and_swap_cache() thing. But it's not exactly urgent.

             Linus
             Linus