[PATCH] arm64: Do not mask out PTE_RDONLY in pte_same()

[PATCH] arm64: Do not mask out PTE_RDONLY in pte_same()

[Catalin Marinas]

Following commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out
of set_pte_at()"), the PTE_RDONLY bit is no longer managed by
set_pte_at() but built into the PAGE_* attribute definitions.
Consequently, pte_same() must include this bit when checking two PTEs
for equality.

Remove the arm64-specific pte_same() function, practically reverting
commit 747a70e60b72 ("arm64: Fix copy-on-write referencing in HugeTLB")

Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
Cc: <stable@vger.kernel.org> # 4.14.x-
Cc: Will Deacon <will@kernel.org>
Cc: Steve Capper <steve.capper@arm.com>
Reported-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---

Steve,

Could you please check that the original problem fixed by commit
747a70e60b72 no longer exists after reverting it in 4.14 onwards?

Thanks.

 arch/arm64/include/asm/pgtable.h | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 5716fe86e7b9..5d15b4735a0e 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -283,23 +283,6 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
 	set_pte(ptep, pte);
 }
 
-#define __HAVE_ARCH_PTE_SAME
-static inline int pte_same(pte_t pte_a, pte_t pte_b)
-{
-	pteval_t lhs, rhs;
-
-	lhs = pte_val(pte_a);
-	rhs = pte_val(pte_b);
-
-	if (pte_present(pte_a))
-		lhs &= ~PTE_RDONLY;
-
-	if (pte_present(pte_b))
-		rhs &= ~PTE_RDONLY;
-
-	return (lhs == rhs);
-}
-
 /*
  * Huge pte definitions.
  */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Do not mask out PTE_RDONLY in pte_same()

[Will Deacon]

On Wed, Nov 06, 2019 at 03:41:05PM +0000, Catalin Marinas wrote:
> Following commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out
> of set_pte_at()"), the PTE_RDONLY bit is no longer managed by
> set_pte_at() but built into the PAGE_* attribute definitions.
> Consequently, pte_same() must include this bit when checking two PTEs
> for equality.
> 
> Remove the arm64-specific pte_same() function, practically reverting
> commit 747a70e60b72 ("arm64: Fix copy-on-write referencing in HugeTLB")
> 
> Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> Cc: <stable@vger.kernel.org> # 4.14.x-
> Cc: Will Deacon <will@kernel.org>
> Cc: Steve Capper <steve.capper@arm.com>
> Reported-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> 
> Steve,
> 
> Could you please check that the original problem fixed by commit
> 747a70e60b72 no longer exists after reverting it in 4.14 onwards?

In the meantime, I've thrown this at the CI to check that they come back
clean.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Do not mask out PTE_RDONLY in pte_same()

[Will Deacon]

On Wed, Nov 06, 2019 at 03:41:05PM +0000, Catalin Marinas wrote:
> Following commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out
> of set_pte_at()"), the PTE_RDONLY bit is no longer managed by
> set_pte_at() but built into the PAGE_* attribute definitions.
> Consequently, pte_same() must include this bit when checking two PTEs
> for equality.
> 
> Remove the arm64-specific pte_same() function, practically reverting
> commit 747a70e60b72 ("arm64: Fix copy-on-write referencing in HugeTLB")
> 
> Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> Cc: <stable@vger.kernel.org> # 4.14.x-
> Cc: Will Deacon <will@kernel.org>
> Cc: Steve Capper <steve.capper@arm.com>
> Reported-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> 
> Steve,
> 
> Could you please check that the original problem fixed by commit
> 747a70e60b72 no longer exists after reverting it in 4.14 onwards?

In the meantime, I've pushed this out to for-next/fixes since the CI came
back clean and it fixes John's issue (which I've confirmed locally too).
Interestingly, I'm not at all sure the problem is related to the Mali
driver. Some tracing suggests that the ART JIT thread is stuck on a write
fault, which could be explained by a broken pte_same().

Steve -- if you could please run the libhugetlbfs test suite as described
in 747a70e60b72 before tomorrow, that would be really great.

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Do not mask out PTE_RDONLY in pte_same()

[Steve Capper]

On Thu, Nov 07, 2019 at 11:48:26AM +0000, Will Deacon wrote:
> On Wed, Nov 06, 2019 at 03:41:05PM +0000, Catalin Marinas wrote:
> > Following commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out
> > of set_pte_at()"), the PTE_RDONLY bit is no longer managed by
> > set_pte_at() but built into the PAGE_* attribute definitions.
> > Consequently, pte_same() must include this bit when checking two PTEs
> > for equality.
> >
> > Remove the arm64-specific pte_same() function, practically reverting
> > commit 747a70e60b72 ("arm64: Fix copy-on-write referencing in HugeTLB")
> >
> > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > Cc: <stable@vger.kernel.org> # 4.14.x-
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Steve Capper <steve.capper@arm.com>
> > Reported-by: John Stultz <john.stultz@linaro.org>
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> >
> > Steve,
> >
> > Could you please check that the original problem fixed by commit
> > 747a70e60b72 no longer exists after reverting it in 4.14 onwards?
>
> In the meantime, I've pushed this out to for-next/fixes since the CI came
> back clean and it fixes John's issue (which I've confirmed locally too).
> Interestingly, I'm not at all sure the problem is related to the Mali
> driver. Some tracing suggests that the ART JIT thread is stuck on a write
> fault, which could be explained by a broken pte_same().
>
> Steve -- if you could please run the libhugetlbfs test suite as described
> in 747a70e60b72 before tomorrow, that would be really great.
>

Hey Will, Catalin,
Apologies for my late reply, I've been travelling longer than originally
planned.

I have tested for-next/fixes (with the pte_same removed), under
libhugetlbfs and I have been unable to reproduce the original memory
leak that prompted the pte_same logic in the first place. So this patch
looks good to me.

Will check this out for 4.14 too now.

Cheers,
--
Steve
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Do not mask out PTE_RDONLY in pte_same()

[Steve Capper]

On Fri, Nov 08, 2019 at 02:08:56AM +0000, Steve Capper wrote:
> On Thu, Nov 07, 2019 at 11:48:26AM +0000, Will Deacon wrote:
> > On Wed, Nov 06, 2019 at 03:41:05PM +0000, Catalin Marinas wrote:
> > > Following commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out
> > > of set_pte_at()"), the PTE_RDONLY bit is no longer managed by
> > > set_pte_at() but built into the PAGE_* attribute definitions.
> > > Consequently, pte_same() must include this bit when checking two PTEs
> > > for equality.
> > >
> > > Remove the arm64-specific pte_same() function, practically reverting
> > > commit 747a70e60b72 ("arm64: Fix copy-on-write referencing in HugeTLB")
> > >
> > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > Cc: Will Deacon <will@kernel.org>
> > > Cc: Steve Capper <steve.capper@arm.com>
> > > Reported-by: John Stultz <john.stultz@linaro.org>
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > ---
> > >
> > > Steve,
> > >
> > > Could you please check that the original problem fixed by commit
> > > 747a70e60b72 no longer exists after reverting it in 4.14 onwards?
> >
> > In the meantime, I've pushed this out to for-next/fixes since the CI came
> > back clean and it fixes John's issue (which I've confirmed locally too).
> > Interestingly, I'm not at all sure the problem is related to the Mali
> > driver. Some tracing suggests that the ART JIT thread is stuck on a write
> > fault, which could be explained by a broken pte_same().
> >
> > Steve -- if you could please run the libhugetlbfs test suite as described
> > in 747a70e60b72 before tomorrow, that would be really great.
> >
> 
> Hey Will, Catalin,
> Apologies for my late reply, I've been travelling longer than originally
> planned.
> 
> I have tested for-next/fixes (with the pte_same removed), under
> libhugetlbfs and I have been unable to reproduce the original memory
> leak that prompted the pte_same logic in the first place. So this patch
> looks good to me.
> 
> Will check this out for 4.14 too now.
>

I can confirm that 4.14 with Catalin's patch cherry-picked also passes
the hugetlbfs cow tests.

Cheers,
-- 
Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Do not mask out PTE_RDONLY in pte_same()

[Catalin Marinas]

On Fri, Nov 08, 2019 at 02:37:04AM +0000, Steve Capper wrote:
> On Fri, Nov 08, 2019 at 02:08:56AM +0000, Steve Capper wrote:
> > On Thu, Nov 07, 2019 at 11:48:26AM +0000, Will Deacon wrote:
> > > On Wed, Nov 06, 2019 at 03:41:05PM +0000, Catalin Marinas wrote:
> > > > Following commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out
> > > > of set_pte_at()"), the PTE_RDONLY bit is no longer managed by
> > > > set_pte_at() but built into the PAGE_* attribute definitions.
> > > > Consequently, pte_same() must include this bit when checking two PTEs
> > > > for equality.
> > > >
> > > > Remove the arm64-specific pte_same() function, practically reverting
> > > > commit 747a70e60b72 ("arm64: Fix copy-on-write referencing in HugeTLB")
> > > >
> > > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > > Cc: Will Deacon <will@kernel.org>
> > > > Cc: Steve Capper <steve.capper@arm.com>
> > > > Reported-by: John Stultz <john.stultz@linaro.org>
> > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > ---
> > > >
> > > > Steve,
> > > >
> > > > Could you please check that the original problem fixed by commit
> > > > 747a70e60b72 no longer exists after reverting it in 4.14 onwards?
> > >
> > > In the meantime, I've pushed this out to for-next/fixes since the CI came
> > > back clean and it fixes John's issue (which I've confirmed locally too).
> > > Interestingly, I'm not at all sure the problem is related to the Mali
> > > driver. Some tracing suggests that the ART JIT thread is stuck on a write
> > > fault, which could be explained by a broken pte_same().
> > >
> > > Steve -- if you could please run the libhugetlbfs test suite as described
> > > in 747a70e60b72 before tomorrow, that would be really great.
> > >
> > 
> > Hey Will, Catalin,
> > Apologies for my late reply, I've been travelling longer than originally
> > planned.
> > 
> > I have tested for-next/fixes (with the pte_same removed), under
> > libhugetlbfs and I have been unable to reproduce the original memory
> > leak that prompted the pte_same logic in the first place. So this patch
> > looks good to me.
> > 
> > Will check this out for 4.14 too now.
> 
> I can confirm that 4.14 with Catalin's patch cherry-picked also passes
> the hugetlbfs cow tests.

Great. Thanks for testing Steve.

The next thing is improving pte_modify() to avoid an unnecessary fault
as we can end up with dirty|write|rdonly pte (and subsequently
ptep_set_access_flags() bailed out earlier due to broken pte_same()).
Anyway, that's not critical now, just minor perf improvement. I'll post
a patch along the lines of https://paste.debian.net/hidden/66246019/.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel