Re: [PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[Jerome Glisse]

On Wed, Aug 09, 2017 at 01:17:42PM -0700, Andrew Morton wrote:
> On Wed, 9 Aug 2017 12:52:46 -0400 (EDT) Jerome Glisse <jglisse@xxxxxxxxxx> wrote:
> 
> > > On Wed, Aug 09, 2017 at 12:17:09PM -0400, jglisse@xxxxxxxxxx wrote:
> > > > From: J__r__me Glisse <jglisse@xxxxxxxxxx>
> > > > 
> > > > MMU notifiers can sleep, but in try_to_unmap_one() we call
> > > > mmu_notifier_invalidate_page() under page table lock.
> > > > 
> > > > Let's instead use mmu_notifier_invalidate_range() outside
> > > > page_vma_mapped_walk() loop.
> > > > 
> > > > Signed-off-by: J__r__me Glisse <jglisse@xxxxxxxxxx>
> > > > Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> > > > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > > > Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use
> > > > page_vma_mapped_walk()")
> > > > ---
> > > >  mm/rmap.c | 36 +++++++++++++++++++++---------------
> > > >  1 file changed, 21 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > > index aff607d5f7d2..d60e887f1cda 100644
> > > > --- a/mm/rmap.c
> > > > +++ b/mm/rmap.c
> > > > @@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page,
> > > > struct vm_area_struct *vma,
> > > >  	};
> > > >  	pte_t pteval;
> > > >  	struct page *subpage;
> > > > -	bool ret = true;
> > > > +	bool ret = true, invalidation_needed = false;
> > > > +	unsigned long end = address + PAGE_SIZE;
> > > 
> > > I think it should be 'address + (1UL << compound_order(page))'.
> > 
> > Can't address point to something else than first page in huge page ?
> > Also i did use end as an optimization ie maybe not all the pte in the
> > range are valid and thus they not all need to be invalidated hence by
> > tracking the last one that needs invalidation i am limiting the range.
> > 
> > But it is a small optimization so i am not attach to it.
> > 
> 
> So we need this patch in addition to Kirrill's "rmap: do not call
> mmu_notifier_invalidate_page() under ptl". 

Yes we need both to restore mmu_notifier.

> 
> Jerome, I'm seeing a bunch of rejects applying this patch to current
> mainline.  It's unclear which kernel you're patching but we'll need
> something which can go into Linus soon and which is backportable (with
> mimimal fixups) into -stable kernels, please.
> 

Sorry this was on top of one of my HMM branches. I am reposting with
Kirill end address computation as it is not a big optimization if pte
are already invalid then invalidating them once more should not trigger
any more work.

Jérôme