From: Anshuman Khandual <firstname.lastname@example.org>
To: Catalin Marinas <email@example.com>
Cc: Will Deacon <firstname.lastname@example.org>,
Steve Capper <email@example.com>,
David Hildenbrand <firstname.lastname@example.org>,
Mike Kravetz <email@example.com>,
"Aneesh Kumar K . V" <firstname.lastname@example.org>,
Peter Zijlstra <email@example.com>,
Subject: Re: VM_BUG_ON(!tlb->end) on munmap() with CONT hugetlb pages
Date: Wed, 11 May 2022 09:12:14 +0530 [thread overview]
Message-ID: <firstname.lastname@example.org> (raw)
On 5/9/22 17:41, Catalin Marinas wrote:
> On Mon, May 09, 2022 at 04:49:25PM +0530, Anshuman Khandual wrote:
>> On 5/6/22 18:19, Will Deacon wrote:
>>> 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
>>>>> 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?
>> Planning to send a patch which drops TLB flushing from get_clear_flush() and
>> also renames it as required. Something like this but just slightly tested.
> The diff looks fine to me. I think this only works if we also have
> commit 697a1d44af8b ("tlb: hugetlb: Add more sizes to
> tlb_remove_huge_tlb_entry"), otherwise we risk missing some TLBIs on the
> unmap path.
FYI, posted here.
prev parent reply other threads:[~2022-05-11 3:41 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-28 15:39 VM_BUG_ON(!tlb->end) on munmap() with CONT hugetlb pages David Hildenbrand
2022-03-01 0:26 ` Mike Kravetz
2022-03-07 23:06 ` Mike Kravetz
2022-03-21 16:34 ` David Hildenbrand
2022-03-22 17:56 ` Catalin Marinas
2022-03-23 11:51 ` Steve Capper
2022-03-23 16:21 ` Catalin Marinas
2022-03-23 16:34 ` Steve Capper
2022-03-23 17:21 ` Mike Kravetz
2022-05-06 12:49 ` Will Deacon
2022-05-09 11:19 ` Anshuman Khandual
2022-05-09 12:11 ` Catalin Marinas
2022-05-11 3:42 ` Anshuman Khandual [this message]
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.