Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <61c5cf99-a962-9a25-597a-53ab1bd8fbc0@google.com>
In-Reply-To: <61c5cf99-a962-9a25-597a-53ab1bd8fbc0@google.com>
From:   Yang Shi <shy828301@gmail.com>
Date:   Thu, 3 Mar 2022 12:12:20 -0800
Message-ID: <CAHbLzkpt8oSsmwFPPpYMPbszNipnAE0OMHnXtMGYBDDrTyP3Cw@mail.gmail.com>
Subject: Re: [PATCH mmotm] mm/thp: ClearPageDoubleMap in first page_add_file_rmap()
To:     Hugh Dickins <hughd@google.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux MM <linux-mm@kvack.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Mar 2, 2022 at 5:50 PM Hugh Dickins <hughd@google.com> wrote:
>
> PageDoubleMap is maintained differently for anon and for shmem+file:
> the shmem+file one was never cleared, because a safe place to do so
> could not be found; so it would blight future use of the cached
> hugepage until evicted.
>
> See https://lore.kernel.org/lkml/1571938066-29031-1-git-send-email-yang.shi@linux.alibaba.com/
>
> But page_add_file_rmap() does provide a safe place to do so (though
> later than one might wish): allowing testing to return to an initial
> state without a damaging drop_caches.
>
> Fixes: 9a73f61bdb8a ("thp, mlock: do not mlock PTE-mapped file huge pages")
> Signed-off-by: Hugh Dickins <hughd@google.com>
> ---
>
>  mm/rmap.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1251,6 +1251,17 @@ void page_add_file_rmap(struct page *page,
>                 }
>                 if (!atomic_inc_and_test(compound_mapcount_ptr(page)))
>                         goto out;
> +
> +               /*
> +                * It is racy to ClearPageDoubleMap in page_remove_file_rmap();
> +                * but page lock is held by all page_add_file_rmap() compound
> +                * callers, and SetPageDoubleMap below warns if !PageLocked:
> +                * so here is a place that DoubleMap can be safely cleared.
> +                */
> +               VM_WARN_ON_ONCE(!PageLocked(page));
> +               if (nr == nr_pages && PageDoubleMap(page))
> +                       ClearPageDoubleMap(page);

Nice idea!

Reviewed-by: Yang Shi <shy828301@gmail.com>

> +
>                 if (PageSwapBacked(page))
>                         __mod_lruvec_page_state(page, NR_SHMEM_PMDMAPPED,
>                                                 nr_pages);