[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f18d39a9-9d81-4435-b4bc-b2dc3b6a74b1@bytedance.com>
Date: Fri, 25 Oct 2024 10:43:47 +0800
From: Qi Zheng <zhengqi.arch@...edance.com>
To: Will Deacon <will@...nel.org>
Cc: Jann Horn <jannh@...gle.com>, Catalin Marinas <catalin.marinas@....com>,
david@...hat.com, hughd@...gle.com, willy@...radead.org, mgorman@...e.de,
muchun.song@...ux.dev, vbabka@...nel.org, akpm@...ux-foundation.org,
zokeefe@...gle.com, rientjes@...gle.com, peterx@...hat.com,
linux-mm@...ck.org, linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH v1 5/7] mm: pgtable: try to reclaim empty PTE page in
madvise(MADV_DONTNEED)
Hi Will,
On 2024/10/24 21:21, Will Deacon wrote:
> On Thu, Oct 17, 2024 at 08:43:43PM +0200, Jann Horn wrote:
>> +arm64 maintainers in case they have opinions on the break-before-make aspects
>
> Thanks, Jann.
>
>> On Thu, Oct 17, 2024 at 11:48 AM Qi Zheng <zhengqi.arch@...edance.com> wrote:
>>> +void try_to_free_pte(struct mm_struct *mm, pmd_t *pmd, unsigned long addr,
>>> + struct mmu_gather *tlb)
>>> +{
>>> + pmd_t pmdval;
>>> + spinlock_t *pml, *ptl;
>>> + pte_t *start_pte, *pte;
>>> + int i;
>>> +
>>> + start_pte = pte_offset_map_rw_nolock(mm, pmd, addr, &pmdval, &ptl);
>>> + if (!start_pte)
>>> + return;
>>> +
>>> + pml = pmd_lock(mm, pmd);
>>> + if (ptl != pml)
>>> + spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
>>> +
>>> + if (unlikely(!pmd_same(pmdval, pmdp_get_lockless(pmd))))
>>> + goto out_ptl;
>>> +
>>> + /* Check if it is empty PTE page */
>>> + for (i = 0, pte = start_pte; i < PTRS_PER_PTE; i++, pte++) {
>>> + if (!pte_none(ptep_get(pte)))
>>> + goto out_ptl;
>>> + }
>>> + pte_unmap(start_pte);
>>> +
>>> + pmd_clear(pmd);
>>> +
>>> + if (ptl != pml)
>>> + spin_unlock(ptl);
>>> + spin_unlock(pml);
>>
>> At this point, you have cleared the PMD and dropped the locks
>> protecting against concurrency, but have not yet done a TLB flush. If
>> another thread concurrently repopulates the PMD at this point, can we
>> get incoherent TLB state in a way that violates the arm64
>> break-before-make rule?
>
> Sounds like it, yes, unless there's something that constrains the new
> PMD value to be some function of what it was in the first place?
Thank you for taking a look at this! I have tried to detect this case
and flush TLB in page fault. For details, please refer to this RFC
patch:
https://lore.kernel.org/lkml/20240815120715.14516-1-zhengqi.arch@bytedance.com/
And more context here:
https://lore.kernel.org/lkml/6f38cb19-9847-4f70-bbe7-06881bb016be@bytedance.com/
If necessary, I can rebase the RFC patch and resend it.
Thanks!
>
> Will
Powered by blists - more mailing lists