[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bbe411f2-4c68-4f92-af8c-da184669dca8@arm.com>
Date: Thu, 1 Aug 2024 15:08:30 +0530
From: Dev Jain <dev.jain@....com>
To: David Hildenbrand <david@...hat.com>, akpm@...ux-foundation.org,
willy@...radead.org
Cc: ryan.roberts@....com, anshuman.khandual@....com, catalin.marinas@....com,
cl@...two.org, vbabka@...e.cz, mhocko@...e.com, apopple@...dia.com,
osalvador@...e.de, baolin.wang@...ux.alibaba.com,
dave.hansen@...ux.intel.com, will@...nel.org, baohua@...nel.org,
ioworker0@...il.com, gshan@...hat.com, mark.rutland@....com,
kirill.shutemov@...ux.intel.com, hughd@...gle.com, aneesh.kumar@...nel.org,
yang@...amperecomputing.com, peterx@...hat.com, broonie@...nel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org
Subject: Re: Race condition observed between page migration and page fault
handling on arm64 machines
On 8/1/24 14:12, David Hildenbrand wrote:
> On 01.08.24 10:16, Dev Jain wrote:
>> I and Ryan had a discussion and we thought it would be best to get
>> feedback
>> from the community.
>>
>> The migration mm selftest currently fails on arm64 for shared anon
>> mappings,
>> due to the following race:
>
> Do you mean MAP_SHARED|MAP_ANON or MAP_PRIVATE|MAP_ANON_fork? Because
> you note shmem below, I assume you mean MAP_SHARED|MAP_ANON
Yes.
>
>>
>> Migration: Page fault:
>> try_to_migrate_one(): handle_pte_fault():
>> 1. Nuke the PTE PTE has been deleted =>
>> do_pte_missing()
>> 2. Mark the PTE for migration PTE has not been deleted
>> but is just not "present" => do_swap_page()
>>
>
> In filemap_fault_recheck_pte_none() we recheck under PTL to make sure
> that a temporary pte_none() really was persistent pte_none() and not a
> temporary pte_none() under PTL.
>
> Should we do something similar in do_fault()? I see that we already do
> something like that on the "!vma->vm_ops->fault" path.
>
> But of course, there is a tradeoff between letting migration
> (temporarily) fail and grabbing the PTL during page faults.
To dampen the tradeoff, we could do this in shmem_fault() instead? But
then, this would mean that we do this in all
kinds of vma->vm_ops->fault, only when we discover another reference
count race condition :) Doing this in do_fault()
should solve this once and for all. In fact, do_pte_missing() may call
do_anonymous_page() or do_fault(), and I just
noticed that the former already checks this using vmf_pte_changed().
Powered by blists - more mailing lists