Re: could you clarify mm/mempolicy: fix !vma in new_vma_page()

[Michal Hocko]

On Wed 08-01-14 20:09:30, Bob Liu wrote:
> On Wed, Jan 8, 2014 at 6:08 PM, Michal Hocko <mhocko@xxxxxxx> wrote:
> 
> >
> > If I was debugging this I would simply add printk into page_address_in_vma
> > error paths.
> >
> > Anyway, I think that at least hugetlbfs part should be reverted because
> > it might paper over real bugs. Although the migration would fail for
> > such hugetlb page we should catch that a weird page was tried to be
> > migrated. What about the patch below?
> 
> Looks good to me. But we need to confirm whether our assumption is right.

Which assumption you have in mind? non-linear mapping or failing on
anon_vma or f_mapping checks?

> Sasha, could you please have a test with Michal's patch?

I obviously doesn't have anything against testing but we should really
focus on the original issue. This patch simply restores hugetlb code
path.

> Thanks,
> -Bob
> 
> > ---
> > From 2d61421f26a3b63b4670d71b7adc67e2191b6157 Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@xxxxxxx>
> > Date: Wed, 8 Jan 2014 10:57:41 +0100
> > Subject: [PATCH] mm: new_vma_page cannot see NULL vma for hugetlb pages
> >
> > 11c731e81bb0 (mm/mempolicy: fix !vma in new_vma_page()) has removed
> > BUG_ON(!vma) from new_vma_page which is partially correct because
> > page_address_in_vma will return EFAULT for non-linear mappings and at
> > least shared shmem might be mapped this way.
> >
> > The patch also tried to prevent NULL ptr for hugetlb pages which is not
> > correct AFAICS because hugetlb pages cannot be mapped as VM_NONLINEAR
> > and other conditions in page_address_in_vma seem to be legit and catch
> > real bugs.
> >
> > This patch restores BUG_ON for PageHuge to catch potential issues when
> > the to-be-migrated page is not setup properly.
> >
> > Signed-off-by: Michal Hocko <mhocko@xxxxxxx>
> > ---
> >  mm/mempolicy.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index 9e8d2d86978a..f3f51464a23b 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -1199,10 +1199,8 @@ static struct page *new_vma_page(struct page *page, unsigned long private, int *
> >         }
> >
> >         if (PageHuge(page)) {
> > -               if (vma)
> > -                       return alloc_huge_page_noerr(vma, address, 1);
> > -               else
> > -                       return NULL;
> > +               BUG_ON(vma)
> > +               return alloc_huge_page_noerr(vma, address, 1);
> >         }
> >         /*
> >          * if !vma, alloc_page_vma() will use task or system default policy
> > --
> > 1.8.5.2
> >
> > --
> > Michal Hocko
> > SUSE Labs

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/