linux-kernel - Re: [PATCH 1/1] mm: do not increment pgfault stats when page fault handler retries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZDmXX82ly2OhfJ1D@casper.infradead.org>
Date:   Fri, 14 Apr 2023 19:11:43 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Suren Baghdasaryan <surenb@...gle.com>
Cc:     akpm@...ux-foundation.org, hannes@...xchg.org, mhocko@...e.com,
        josef@...icpanda.com, jack@...e.cz, ldufour@...ux.ibm.com,
        laurent.dufour@...ibm.com, michel@...pinasse.org,
        liam.howlett@...cle.com, jglisse@...gle.com, vbabka@...e.cz,
        minchan@...gle.com, dave@...olabs.net, punit.agrawal@...edance.com,
        lstoakes@...il.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH 1/1] mm: do not increment pgfault stats when page fault
 handler retries

On Fri, Apr 14, 2023 at 10:54:44AM -0700, Suren Baghdasaryan wrote:
> If the page fault handler requests a retry, we will count the fault
> multiple times.  This is a relatively harmless problem as the retry paths
> are not often requested, and the only user-visible problem is that the
> fault counter will be slightly higher than it should be.  Nevertheless,
> userspace only took one fault, and should not see the fact that the
> kernel had to retry the fault multiple times.
> 
> Fixes: 6b4c9f446981 ("filemap: drop the mmap_sem for all blocking operations")

I know I suggested this fixes line, but I think it's actually been
here much longer, perhaps since

Fixes: d065bd810b6d ("mm: retry page fault when blocking on disk transfer")

Michel, what do you think?

> Signed-off-by: Suren Baghdasaryan <surenb@...gle.com>
> Reviewed-by: Matthew Wilcox (Oracle) <willy@...radead.org>
> ---
> Patch applies cleanly over linux-next and mm-unstable
> 
>  mm/memory.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 1c5b231fe6e3..d88f370eacd1 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5212,17 +5212,16 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
>  
>  	__set_current_state(TASK_RUNNING);
>  
> -	count_vm_event(PGFAULT);
> -	count_memcg_event_mm(vma->vm_mm, PGFAULT);
> -
>  	ret = sanitize_fault_flags(vma, &flags);
>  	if (ret)
> -		return ret;
> +		goto out;
>  
>  	if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
>  					    flags & FAULT_FLAG_INSTRUCTION,
> -					    flags & FAULT_FLAG_REMOTE))
> -		return VM_FAULT_SIGSEGV;
> +					    flags & FAULT_FLAG_REMOTE)) {
> +		ret = VM_FAULT_SIGSEGV;
> +		goto out;
> +	}
>  
>  	/*
>  	 * Enable the memcg OOM handling for faults triggered in user
> @@ -5253,6 +5252,11 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
>  	}
>  
>  	mm_account_fault(regs, address, flags, ret);
> +out:
> +	if (!(ret & VM_FAULT_RETRY)) {
> +		count_vm_event(PGFAULT);
> +		count_memcg_event_mm(vma->vm_mm, PGFAULT);
> +	}
>  
>  	return ret;
>  }
> -- 
> 2.40.0.634.g4ca3ef3211-goog
>