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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHk-=wjg66VL1=F74-YDndwN2FvfUDG30XMgG7USiNkmExDeZA@mail.gmail.com>
Date:   Tue, 1 Jun 2021 17:18:54 -1000
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Johannes Weiner <hannes@...xchg.org>
Cc:     Matthew Wilcox <willy@...radead.org>,
        Huang Ying <ying.huang@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux-MM <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Peter Xu <peterx@...hat.com>, Hugh Dickins <hughd@...gle.com>,
        Mel Gorman <mgorman@...e.de>, Rik van Riel <riel@...riel.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Michal Hocko <mhocko@...nel.org>,
        Dave Hansen <dave.hansen@...el.com>,
        Tim Chen <tim.c.chen@...el.com>
Subject: Re: [PATCH] mm: free idle swap cache page after COW

On Tue, Jun 1, 2021 at 5:00 AM Johannes Weiner <hannes@...xchg.org> 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