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>
In-Reply-To: <c3f34084-7315-e0c5-55db-d1cb006979f4@redhat.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu, 13 Jan 2022 09:44:04 -0800
Message-ID: <CAHk-=wjv+beg2gRNdERANGfaGcqwDzzVD5RDD07FcrE5c6k-XA@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:25 AM David Hildenbrand <david@redhat.com> wrote:
>
> I might be missing something, but it's not only about whether we can remove
> the page from the swap cache, it's about whether we can reuse the page
> exclusively in a process with write access, avoiding a COW. And for that we
> have to check if it's mapped somewhere else already (readable).

No.

The "try to remove from swap cache" is one thing. That uses the swap count.

The "see if we can reuse this page for COW" is a completely different
test, and that's the "page_count() == 1" one.

The two should not be mixed up with each other. Just don't do it.
There's no reason - except for legacy confusion that should be
actively avoided and removed.

IOW, the COW path would do

 trylock - copy if fails
 try to remove from swap cache
 if page_count() is now 1, we can reuse it

Note how the "try to remove from swap cache" is entirely independent
of whether we then reuse it or not.

And yes, we can have optimistic other tests - like not even bothering
to trylock if we can see that the page-count is so elevated that it
makes no difference and trying to remove from swap cache would be just
pointless extra work (both the removal itself, and then potentially
later re-additions).

But those should be seen for what they are - not important for
semantics, only a "don't bother, this page has so many users that we
already know that removing the swapcache one doesn't make any
difference at all".

Now, it's possible that I'm missing something, but I think this kind
of clarity is very much what we should aim for. Clear rules, no mixing
of "can I COW this" with "can I remove this from the swap cache".

And now I need to start my travel nightmare, so I'll be effectively
offline for the next 24 hours ;(

              Linus