linux-kernel - Re: [PATCH] mm: reuse the unshared swapcache page in do_wp

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wg-FvLd9Vyg1+n47gDSSDmHJnjmW0-FaQGJcbrtgbYALg@mail.gmail.com>
Date:   Thu, 13 Jan 2022 10:55:06 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     David Hildenbrand <david@...hat.com>
Cc:     Matthew Wilcox <willy@...radead.org>,
        Liang Zhang <zhangliang5@...wei.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux-MM <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        wangzhigang17@...wei.com
Subject: Re: [PATCH] mm: reuse the unshared swapcache page in do_wp_page

On Thu, Jan 13, 2022 at 9:55 AM David Hildenbrand <david@...hat.com> wrote:
>
> However, reuse_swap_page() currently does multiple things, and that's part of the issue.

Yeah, I think it's a horrible function. The COW path was explicitly
changed not to use it because it's so confusing.

> While we're at it, is there a real reason we can't simplify to
>
> diff --git a/mm/memory.c b/mm/memory.c
> index e8e2144cbfa6..ab114a5862a0 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3295,7 +3295,7 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf)
> -               if (PageKsm(page) || page_mapcount(page) != 1 || page_count(page) != 1) {
> +               if (PageKsm(page) || page_count(page) != 1) {

No, I think both the mapcount and the ksm check are bogus and should
be removed, but they are both examples of "get rid of the nastiest
complexity, leave the stuff that doesn't much matter".

Somebody who really knows the ksm logic should check all the ksm
tests. I really think they are pointless, and came from the old and
horribly broken page_mapcount() logic that did *not* check
page_count().

When you think page_mapcount() matters, suddenly you have to worry
about all the other non-mapping uses of a page, which is why I think
that ksm test still exists. But I didn't mind keeping a couple of
extraneous tests that I didn't see the point of , as long as the core
page-count-based logfc was solid

          Linus