On Wed, 06 Jul 2022 20:42:56 -0400 Rik van Riel <riel@xxxxxxxxxxx> wrote:
> On Thu, 2022-07-07 at 01:46 +0300, Kirill A. Shutemov wrote:
> > On Tue, Jul 05, 2022 at 04:00:36PM -0400, Josef Bacik wrote:
> > >
> > > Fix this by returning VM_FAULT_NOPAGE in the
> > > pmd_devmap_trans_unstable()
> > > case, this makes us drop the ref on the page properly, and now my
> > > reproducer no longer leaks the huge pages.
> > >
> > > Fixes: f9ce0be71d1f ("mm: Cleanup faultaround and finish_fault()
> > > codepaths")
> > > Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> > > Cc: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > > Signed-off-by: Rik van Riel <riel@xxxxxxxxxxx>
> > > Signed-off-by: Chris Mason <clm@xxxxxx>
> >
> > Cc: stable@
>
> Yes, it should. How do we send a patch to stable@
> after the start of the thread?
cc'ing the stable list doesn't have much affect, afaik.
What Kirill means is that we should add cc:stable to this patch's
changelog. I did that yesterday.
>
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -4371,7 +4371,7 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
> > >
> > > /* See comment in handle_pte_fault() */
> > > if (pmd_devmap_trans_unstable(vmf->pmd))
> > > - return 0;
> > > + return VM_FAULT_NOPAGE;
> >
> > Comment update would be nice.
> >
> > Other instances of pmd_devmap_trans_unstable() return 0 in the fault
> > path.
> > Explanation would be helpful.
> >
> 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!
