Re: [PATCH] rmap: fix pgoff calculation to handle hugepage correctly

[Naoya Horiguchi]

On Tue, Jul 01, 2014 at 09:07:39PM +0300, Kirill A. Shutemov wrote:
> On Tue, Jul 01, 2014 at 10:46:22AM -0400, Naoya Horiguchi wrote:
> > I triggered VM_BUG_ON() in vma_address() when I try to migrate an anonymous
> > hugepage with mbind() in the kernel v3.16-rc3. This is because pgoff's
> > calculation in rmap_walk_anon() fails to consider compound_order() only to
> > have an incorrect value. So this patch fixes it.
> > 
> > Signed-off-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
> > ---
> >  mm/rmap.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git v3.16-rc3.orig/mm/rmap.c v3.16-rc3/mm/rmap.c
> > index b7e94ebbd09e..8cc964c6bd8d 100644
> > --- v3.16-rc3.orig/mm/rmap.c
> > +++ v3.16-rc3/mm/rmap.c
> > @@ -1639,7 +1639,7 @@ static struct anon_vma *rmap_walk_anon_lock(struct page *page,
> >  static int rmap_walk_anon(struct page *page, struct rmap_walk_control *rwc)
> >  {
> >  	struct anon_vma *anon_vma;
> > -	pgoff_t pgoff = page->index << (PAGE_CACHE_SHIFT - PAGE_SHIFT);
> > +	pgoff_t pgoff = page->index << compound_order(page);
> >  	struct anon_vma_chain *avc;
> >  	int ret = SWAP_AGAIN;
> 
> Hm. It will not work with THP: ->index there is in PAGE_SIZE units.

I wrongly assumed that rmap is never used by thp, sorry.

> Why do we need this special case for hugetlb page ->index? Why not use
> PAGE_SIZE units there too? Or I miss something?

hugetlb pages are never split, so we use larger page cache size for
hugetlbfs file (to avoid large sparse page cache tree.) I'm not sure
if we should do this for anonymous hugepages, but I guess that using
different cache size in hugetlbfs makes code complicated.

Anyway I'll do some generalization to handle any types of pages
rmap_walk_anon() can called on. Maybe something similar to
linear_page_index() will be added.

Thanks,
Naoya Horiguchi
--
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/