Re: [PATCH] mm: fix page leak with multiple threads mapping the same page

[Rik van Riel <riel@surriel.com>]

[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

On Thu, 2022-07-07 at 17:58 -0700, Andrew Morton wrote:
> On Wed, 06 Jul 2022 20:42:56 -0400 Rik van Riel <riel@surriel.com>
> wrote:
> > > 
> > The explanation is that by the time we get to
> > finish_fault, we already have a reference on a
> > page, and we need to ensure that reference
> > gets released by the caller.
> > 
> > VM_FAULT_NOPAGE is one of the ways to indicate
> > that the page should be freed.
> 
> I think Kirill meant this should be added to the code comment!

OK, I finally got around to looking at that today, and
it seems like the comment at the top of finish_fault()
already has everything I wanted to write down. Is there
anything else we should add here?

/**
 * finish_fault - finish page fault once we have prepared the page to
fault
 *
 * @vmf: structure describing the fault
 *
 * This function handles all that is needed to finish a page fault once
the
 * page to fault in is prepared. It handles locking of PTEs, inserts
PTE for
 * given page, adds reverse page mapping, handles memcg charges and LRU
 * addition.
 *
 * The function expects the page to be locked and on success it
consumes a
 * reference of a page being mapped (for the PTE which maps it).
 *
 * Return: %0 on success, %VM_FAULT_ code in case of error.
 */
vm_fault_t finish_fault(struct vm_fault *vmf)
{



-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]