linux-kernel - Re: [PATCH -V2] mm: move idle swap cache pages to the tail of LRU 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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 27 May 2021 15:27:28 -1000
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Johannes Weiner <hannes@...xchg.org>
Cc:     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>,
        Matthew Wilcox <willy@...radead.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 -V2] mm: move idle swap cache pages to the tail of LRU
 after COW

On Thu, May 27, 2021 at 6:53 AM Johannes Weiner <hannes@...xchg.org> wrote:
>
> OTOH, freeing is a two-liner reusing the swap unmap code:
>
>         if (page_copied)
>                 free_swap_cache(old_page);
>
> Linus, what do you think?

I'm ok with that version, the important thing was

 (a) avoiding the unconditional page lock we used to have (well, it
first did "trylock", but if tht failed it would then get a page ref,
and do the unconditional lock_page())

 (b) avoid the re-use based on "mapcount" that had problems with
non-mapped page references (ie GUP)

and

 (c) that I wanted to see some numbers rather than just blindly
re-introduce free_swap_cache()

But doing the above two-liner in wp_page_copy() doesn't have the
(a)/(b) issues, and if we have numbers that it helps, then that takes
care of (c) too.

Of course, I don't think it's just that two-liner, because you'd
actually have to export (or move )that "free_swap_cache()" function
that is now private to swapfile.c.

But no, I'm not adverse to the above at all, I just had the above reservations.

I was worried about non-swap behavior (which the old code had with
that whole unconditional page locking whether needed or not), but
free_swap_cache() should be basically free for the non-swap behavior
since it doesn't even do the trylock until after it has checked that
it is now an unmapped swap cache page.

              Linus