linux-kernel - Re: [PATCH AUTOSEL 13/14] mm/thp: ClearPageDoubleMap in first page_add_file

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: <c2ed1fe1-247e-e644-c367-87d32eb92cf5@google.com>
Date:   Thu, 28 Apr 2022 09:51:58 -0700 (PDT)
From:   Hugh Dickins <hughd@...gle.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
cc:     stable@...r.kernel.org, linux-kernel@...r.kernel.org,
        Hugh Dickins <hughd@...gle.com>,
        Yang Shi <shy828301@...il.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-mm@...ck.org
Subject: Re: [PATCH AUTOSEL 13/14] mm/thp: ClearPageDoubleMap in first
 page_add_file_rmap()

On Thu, 28 Apr 2022, Greg Kroah-Hartman wrote:

> From: Hugh Dickins <hughd@...gle.com>
> 
> commit bd55b0c2d64e84a75575f548a33a3dfecc135b65 upstream.
> 
> 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.
> 
> Link: https://lkml.kernel.org/r/61c5cf99-a962-9a25-597a-53ab1bd8fbc0@google.com
> Fixes: 9a73f61bdb8a ("thp, mlock: do not mlock PTE-mapped file huge pages")
> Signed-off-by: Hugh Dickins <hughd@...gle.com>
> Reviewed-by: Yang Shi <shy828301@...il.com>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

NAK.

I thought we had a long-standing agreement that AUTOSEL does not try
to add patches from akpm's tree which had not been marked for stable.

(Whereas, if a developer asks for such a patch to be added to stable
later, and verifies the result, that's of course a different matter.)

I've chosen to answer to this patch of my 3 in your 14 AUTOSELs,
because this one is just an improvement, not at all a bugfix needed
for stable (maybe AUTOSEL noticed "racy" or "safely" in the comments,
and misunderstood).  The "Fixes" was intended to help any humans who
wanted to backport into their trees.

I do recall that this 13/14, and 14/14, are mods to mm/rmap.c
which followed other (mm/munlock) mods to mm/rmap.c in 5.18-rc1,
which affected the out path of the function involved, and somehow
made 14/14 a little cleaner.  I'm sorry, but I just don't rate it
worth my time at the moment, to verify whether 14/14 happens to
have ended up as a correct patch or not.

And nobody can verify them without these AUTOSELs saying to which
tree they are targeted - 5.17 I suppose.

Hugh

> ---
>  mm/rmap.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 9e27f9f038d3..444d0d958aff 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1252,6 +1252,17 @@ void page_add_file_rmap(struct page *page, bool compound)
>  		}
>  		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);
> +
>  		if (PageSwapBacked(page))
>  			__mod_lruvec_page_state(page, NR_SHMEM_PMDMAPPED,
>  						nr_pages);
> -- 
> 2.36.0