[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7b2f7f85-4790-4eeb-adea-6ff1d399bd28@kernel.org>
Date: Thu, 11 Dec 2025 02:58:51 +0100
From: "David Hildenbrand (Red Hat)" <david@...nel.org>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
linux-mm@...ck.org, Will Deacon <will@...nel.org>,
"Aneesh Kumar K.V" <aneesh.kumar@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>, Nick Piggin <npiggin@...il.com>,
Peter Zijlstra <peterz@...radead.org>, Arnd Bergmann <arnd@...db.de>,
Muchun Song <muchun.song@...ux.dev>, Oscar Salvador <osalvador@...e.de>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>, Vlastimil Babka
<vbabka@...e.cz>, Jann Horn <jannh@...gle.com>,
Pedro Falcato <pfalcato@...e.de>, Rik van Riel <riel@...riel.com>,
Harry Yoo <harry.yoo@...cle.com>, Laurence Oberman <loberman@...hat.com>,
Prakash Sangappa <prakash.sangappa@...cle.com>,
Nadav Amit <nadav.amit@...il.com>, Liu Shixin <liushixin2@...wei.com>
Subject: Re: [PATCH v1 2/4] mm/hugetlb: fix two comments related to
huge_pmd_unshare()
On 12/10/25 12:22, Lorenzo Stoakes wrote:
> On Fri, Dec 05, 2025 at 10:35:56PM +0100, David Hildenbrand (Red Hat) wrote:
>> Ever since we stopped using the page count to detect shared PMD
>> page tables, these comments are outdated.
>>
>> The only reason we have to flush the TLB early is because once we drop
>> the i_mmap_rwsem, the previously shared page table could get freed (to
>> then get reallocated and used for other purpose). So we really have to
>> flush the TLB before that could happen.
>>
>> So let's simplify the comments a bit.
>>
>> The "If we unshared PMDs, the TLB flush was not recorded in mmu_gather."
>> part introduced as in commit a4a118f2eead ("hugetlbfs: flush TLBs
>> correctly after huge_pmd_unshare") was confusing: sure it is recorded
>> in the mmu_gather, otherwise tlb_flush_mmu_tlbonly() wouldn't do
>> anything. So let's drop that comment while at it as well.
>>
>> We'll centralize these comments in a single helper as we rework the code
>> next.
>>
>> Fixes: 59d9094df3d7 ("mm: hugetlb: independent PMD page table shared count")
>> Cc: Liu Shixin <liushixin2@...wei.com>
>> Signed-off-by: David Hildenbrand (Red Hat) <david@...nel.org>
>
> LGTM, so:
>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Thanks!
>
>> ---
>> mm/hugetlb.c | 24 ++++++++----------------
>> 1 file changed, 8 insertions(+), 16 deletions(-)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 51273baec9e5d..3c77cdef12a32 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -5304,17 +5304,10 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
>> tlb_end_vma(tlb, vma);
>>
>> /*
>> - * If we unshared PMDs, the TLB flush was not recorded in mmu_gather. We
>> - * could defer the flush until now, since by holding i_mmap_rwsem we
>> - * guaranteed that the last reference would not be dropped. But we must
>> - * do the flushing before we return, as otherwise i_mmap_rwsem will be
>> - * dropped and the last reference to the shared PMDs page might be
>> - * dropped as well.
>> - *
>> - * In theory we could defer the freeing of the PMD pages as well, but
>> - * huge_pmd_unshare() relies on the exact page_count for the PMD page to
>> - * detect sharing, so we cannot defer the release of the page either.
>
> Was it this comment that led you to question the page_count issue? :)
Heh, no, I know about the changed handling already. I stumbled over the
page_count() remaining usage while working on some cleanups I previously
had as part of this series :)
--
Cheers
David
Powered by blists - more mailing lists