Re: VM_BUG_ON(!tlb->end) on munmap() with CONT hugetlb pages

From: Will Deacon <will@kernel.org>
To: Steve Capper <steve.capper@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	David Hildenbrand <david@redhat.com>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	anshuman.khandual@arm.com,
	"Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	nd@arm.com
Subject: Re: VM_BUG_ON(!tlb->end) on munmap() with CONT hugetlb pages
Date: Fri, 6 May 2022 13:49:11 +0100	[thread overview]
Message-ID: <20220506124909.GA22892@willie-the-truck> (raw)
In-Reply-To: <881efece-c362-af41-4dea-77db71ec9928@arm.com>

On Wed, Mar 23, 2022 at 04:34:26PM +0000, Steve Capper wrote:
> 
> 
> On 23/03/2022 16:21, Catalin Marinas wrote:
> > On Wed, Mar 23, 2022 at 11:51:25AM +0000, Steve Capper wrote:
> > > On 22/03/2022 17:56, Catalin Marinas wrote:
> > > > At a quick look, we wouldn't have a problem with missing TLB flushing
> > > > since huge_ptep_get_and_clear() does this for contiguous PTEs. Not sure
> > > > why it needs this though, Steve added it in commit d8bdcff28764. I think
> > > > we can defer this flushing to tlb_remove_page_size().
> > > 
> > > The TLB flush in huge_ptep_get_and_clear() was added because it was called
> > > by hugetlb_change_protection() without any flushing. The concern was that,
> > > without the flush, it would be possible to get to different views of the
> > > same contiguous huge page. (Being contiguous they were not changed en masse
> > > atomically).
> > 
> > Maybe the code paths have been changed since but looking at
> > hugetlb_change_protection(), we have huge_ptep_modify_prot_start()
> > calling huge_ptep_get_and_clear() which AFAICT only needs to clear the
> > ptes. huge_ptep_modify_prot_commit() calls set_huge_pte_at() which does
> > another pte clearing + TLBI (clear_flush()) before setting the new ptes.
> > So we do the pte clearing and TLBI twice already.
> > 
> 
> Thanks, yeah indeed the code has changed and the flush should be removed
> from the arm64 huge_ptep_get_and_clear.

Did anybody send a patch for this?

Will