MIME-Version: 1.0
References: <20220113140318.11117-1-zhangliang5@huawei.com>
 <YeA5oP/iaxtVPHb3@casper.infradead.org> <CAHk-=wjB0i-B=U-DhpAajQx3f6bp1X==neLOrg0jwq29mgz=3g@mail.gmail.com>
 <172ccfbb-7e24-db21-7d84-8c8d8c3805fd@redhat.com> <CAHk-=wi21DZ4H5uLnn2QgAeAUqg0wNPboijC0OgDDk1e7TdkPw@mail.gmail.com>
 <c3f34084-7315-e0c5-55db-d1cb006979f4@redhat.com> <CAHk-=wjv+beg2gRNdERANGfaGcqwDzzVD5RDD07FcrE5c6k-XA@mail.gmail.com>
 <0c44a89d-06a7-d0bb-e71e-7947d651f4d1@redhat.com>
In-Reply-To: <0c44a89d-06a7-d0bb-e71e-7947d651f4d1@redhat.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu, 13 Jan 2022 10:55:06 -0800
Message-ID: <CAHk-=wg-FvLd9Vyg1+n47gDSSDmHJnjmW0-FaQGJcbrtgbYALg@mail.gmail.com>
Subject: Re: [PATCH] mm: reuse the unshared swapcache page in do_wp_page
To:     David Hildenbrand <david@redhat.com>
Cc:     Matthew Wilcox <willy@infradead.org>,
        Liang Zhang <zhangliang5@huawei.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux-MM <linux-mm@kvack.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        wangzhigang17@huawei.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jan 13, 2022 at 9:55 AM David Hildenbrand <david@redhat.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