[PATCH AUTOSEL 5.10 2/8] parisc: Disable debug code regarding cache flushes in handle_nadtlb_fault()

[PATCH AUTOSEL 5.10 2/8] parisc: Disable debug code regarding cache flushes in handle_nadtlb_fault()

[Sasha Levin]

From: John David Anglin <dave.anglin@bell.net>

[ Upstream commit 67c35a3b646cc68598ff0bb28de5f8bd7b2e81b3 ]

Change the "BUG" to "WARNING" and disable the message because it triggers
occasionally in spite of the check in flush_cache_page_if_present.

The pte value extracted for the "from" page in copy_user_highpage is racy and
occasionally the pte is cleared before the flush is complete.  I assume that
the page is simultaneously flushed by flush_cache_mm before the pte is cleared
as nullifying the fdc doesn't seem to cause problems.

I investigated various locking scenarios but I wasn't able to find a way to
sequence the flushes.  This code is called for every COW break and locks impact
performance.

This patch is related to the bigger cache flush patch because we need the pte
on PA8800/PA8900 to flush using the vma context.
I have also seen this from copy_to_user_page and copy_from_user_page.

The messages appear infrequently when enabled.

Signed-off-by: John David Anglin <dave.anglin@bell.net>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/parisc/mm/fault.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/parisc/mm/fault.c b/arch/parisc/mm/fault.c
index 5faa3cff4738..2472780d4039 100644
--- a/arch/parisc/mm/fault.c
+++ b/arch/parisc/mm/fault.c
@@ -22,6 +22,8 @@
 
 #include <asm/traps.h>
 
+#define DEBUG_NATLB 0
+
 /* Various important other fields */
 #define bit22set(x)		(x & 0x00000200)
 #define bits23_25set(x)		(x & 0x000001c0)
@@ -449,8 +451,8 @@ handle_nadtlb_fault(struct pt_regs *regs)
 		fallthrough;
 	case 0x380:
 		/* PDC and FIC instructions */
-		if (printk_ratelimit()) {
-			pr_warn("BUG: nullifying cache flush/purge instruction\n");
+		if (DEBUG_NATLB && printk_ratelimit()) {
+			pr_warn("WARNING: nullifying cache flush/purge instruction\n");
 			show_regs(regs);
 		}
 		if (insn & 0x20) {
-- 
2.35.1

Re: [PATCH AUTOSEL 5.10 2/8] parisc: Disable debug code regarding cache flushes in handle_nadtlb_fault()

[Helge Deller]

Hello Sascha,

On 5/24/22 18:00, Sasha Levin wrote:
> From: John David Anglin <dave.anglin@bell.net>
>
> [ Upstream commit 67c35a3b646cc68598ff0bb28de5f8bd7b2e81b3 ]
>
> Change the "BUG" to "WARNING" and disable the message because it triggers
> occasionally in spite of the check in flush_cache_page_if_present.

Please drop this patch from the backporting-queue (v5.10, v5.15 and v5.17).
It's not necessary since the warning will only trigger on v5.18 on machines
with PA8800/PA8900 processors.

Thanks.
Helge


> The pte value extracted for the "from" page in copy_user_highpage is racy and
> occasionally the pte is cleared before the flush is complete.  I assume that
> the page is simultaneously flushed by flush_cache_mm before the pte is cleared
> as nullifying the fdc doesn't seem to cause problems.
>
> I investigated various locking scenarios but I wasn't able to find a way to
> sequence the flushes.  This code is called for every COW break and locks impact
> performance.
>
> This patch is related to the bigger cache flush patch because we need the pte
> on PA8800/PA8900 to flush using the vma context.
> I have also seen this from copy_to_user_page and copy_from_user_page.
>
> The messages appear infrequently when enabled.
>
> Signed-off-by: John David Anglin <dave.anglin@bell.net>
> Signed-off-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  arch/parisc/mm/fault.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/parisc/mm/fault.c b/arch/parisc/mm/fault.c
> index 5faa3cff4738..2472780d4039 100644
> --- a/arch/parisc/mm/fault.c
> +++ b/arch/parisc/mm/fault.c
> @@ -22,6 +22,8 @@
>
>  #include <asm/traps.h>
>
> +#define DEBUG_NATLB 0
> +
>  /* Various important other fields */
>  #define bit22set(x)		(x & 0x00000200)
>  #define bits23_25set(x)		(x & 0x000001c0)
> @@ -449,8 +451,8 @@ handle_nadtlb_fault(struct pt_regs *regs)
>  		fallthrough;
>  	case 0x380:
>  		/* PDC and FIC instructions */
> -		if (printk_ratelimit()) {
> -			pr_warn("BUG: nullifying cache flush/purge instruction\n");
> +		if (DEBUG_NATLB && printk_ratelimit()) {
> +			pr_warn("WARNING: nullifying cache flush/purge instruction\n");
>  			show_regs(regs);
>  		}
>  		if (insn & 0x20) {

Re: [PATCH AUTOSEL 5.10 2/8] parisc: Disable debug code regarding cache flushes in handle_nadtlb_fault()

[Sasha Levin]

On Tue, May 24, 2022 at 06:44:59PM +0200, Helge Deller wrote:
>Hello Sascha,
>
>On 5/24/22 18:00, Sasha Levin wrote:
>> From: John David Anglin <dave.anglin@bell.net>
>>
>> [ Upstream commit 67c35a3b646cc68598ff0bb28de5f8bd7b2e81b3 ]
>>
>> Change the "BUG" to "WARNING" and disable the message because it triggers
>> occasionally in spite of the check in flush_cache_page_if_present.
>
>Please drop this patch from the backporting-queue (v5.10, v5.15 and v5.17).
>It's not necessary since the warning will only trigger on v5.18 on machines
>with PA8800/PA8900 processors.

Will do, thanks.

-- 
Thanks,
Sasha