mbox series

[v12,0/4] fix double page fault in cow_user_page for pfn mapping

Message ID 20191011140939.6115-1-justin.he@arm.com (mailing list archive)
Headers show
Series fix double page fault in cow_user_page for pfn mapping | expand

Message

Justin He Oct. 11, 2019, 2:09 p.m. UTC
When we tested pmdk unit test vmmalloc_fork TEST1 in arm64 guest, there
will be a double page fault in __copy_from_user_inatomic of cow_user_page.

As told by Catalin: "On arm64 without hardware Access Flag, copying from
user will fail because the pte is old and cannot be marked young. So we
always end up with zeroed page after fork() + CoW for pfn mappings. we
don't always have a hardware-managed access flag on arm64."

-Changes
v12:
    refine PATCH 01, remove the !! since C languages can convert unsigned
    to bool (Catalin)
v11:
    refine cpu_has_hw_af in PATCH 01(Will Deacon, Suzuki)
    change the default return value to true in arch_faults_on_old_pte
    add PATCH 03 for overriding arch_faults_on_old_pte(false) on x86
v10:
    add r-b from Catalin and a-b from Kirill in PATCH 03
    remoe Reported-by in PATCH 01
v9: refactor cow_user_page for indention optimization (Catalin)
    hold the ptl longer (Catalin)
v8: change cow_user_page's return type (Matthew)
v7: s/pte_spinlock/pte_offset_map_lock (Kirill)
v6: fix error case of returning with spinlock taken (Catalin)
    move kmap_atomic to avoid handling kunmap_atomic
v5: handle the case correctly when !pte_same
    fix kbuild test failed
v4: introduce cpu_has_hw_af (Suzuki)
    bail out if !pte_same (Kirill)
v3: add vmf->ptl lock/unlock (Kirill A. Shutemov)
    add arch_faults_on_old_pte (Matthew, Catalin)
v2: remove FAULT_FLAG_WRITE when setting pte access flag (Catalin)

Jia He (4):
  arm64: cpufeature: introduce helper cpu_has_hw_af()
  arm64: mm: implement arch_faults_on_old_pte() on arm64
  x86/mm: implement arch_faults_on_old_pte() stub on x86
  mm: fix double page fault on arm64 if PTE_AF is cleared

 arch/arm64/include/asm/cpufeature.h |  14 ++++
 arch/arm64/include/asm/pgtable.h    |  14 ++++
 arch/x86/include/asm/pgtable.h      |   6 ++
 mm/memory.c                         | 104 ++++++++++++++++++++++++----
 4 files changed, 123 insertions(+), 15 deletions(-)

Comments

Will Deacon Oct. 15, 2019, 12:18 a.m. UTC | #1
On Fri, Oct 11, 2019 at 10:09:35PM +0800, Jia He wrote:
> When we tested pmdk unit test vmmalloc_fork TEST1 in arm64 guest, there
> will be a double page fault in __copy_from_user_inatomic of cow_user_page.
> 
> As told by Catalin: "On arm64 without hardware Access Flag, copying from
> user will fail because the pte is old and cannot be marked young. So we
> always end up with zeroed page after fork() + CoW for pfn mappings. we
> don't always have a hardware-managed access flag on arm64."
> 
> -Changes
> v12:
>     refine PATCH 01, remove the !! since C languages can convert unsigned
>     to bool (Catalin)

Thanks. I think it's a bit late to take something like this for 5.4 now,
especially as the current behaviour has always been there. Hopefully
somebody can queue it for 5.5 instead.

Will
Catalin Marinas Oct. 16, 2019, 2:32 p.m. UTC | #2
On Tue, Oct 15, 2019 at 01:18:34AM +0100, Will Deacon wrote:
> On Fri, Oct 11, 2019 at 10:09:35PM +0800, Jia He wrote:
> > When we tested pmdk unit test vmmalloc_fork TEST1 in arm64 guest, there
> > will be a double page fault in __copy_from_user_inatomic of cow_user_page.
> > 
> > As told by Catalin: "On arm64 without hardware Access Flag, copying from
> > user will fail because the pte is old and cannot be marked young. So we
> > always end up with zeroed page after fork() + CoW for pfn mappings. we
> > don't always have a hardware-managed access flag on arm64."
> > 
> > -Changes
> > v12:
> >     refine PATCH 01, remove the !! since C languages can convert unsigned
> >     to bool (Catalin)
> 
> Thanks. I think it's a bit late to take something like this for 5.4 now,
> especially as the current behaviour has always been there. Hopefully
> somebody can queue it for 5.5 instead.

I can queue this through the arm64 tree for 5.5 if I get an ack on the
x86 patch (3/4) or I don't hear any complaints.