On Wed, Mar 24, 2010 at 02:57:25PM +0900, KAMEZAWA Hiroyuki wrote:
> On Wed, 24 Mar 2010 14:42:27 +0900
> Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx> wrote:
>
> > When we look into pagemap using page-types with option -p, the value
> > of pfn for hugepages looks wrong (see below.)
> > This is because pte was evaluated only once for one vma
> > although it should be updated for each hugepage. This patch fixes it.
> >
> > $ page-types -p 3277 -Nl -b huge
> > voffset offset len flags
> > 7f21e8a00 11e400 1 ___U___________H_G________________
> > 7f21e8a01 11e401 1ff ________________TG________________
> > ^^^
> > 7f21e8c00 11e400 1 ___U___________H_G________________
> > 7f21e8c01 11e401 1ff ________________TG________________
> > ^^^
> >
> > One hugepage contains 1 head page and 511 tail pages in x86_64 and
> > each two lines represent each hugepage. Voffset and offset mean
> > virtual address and physical address in the page unit, respectively.
> > The different hugepages should not have the same offset value.
> >
> > With this patch applied:
> >
> > $ page-types -p 3386 -Nl -b huge
> > voffset offset len flags
> > 7fec7a600 112c00 1 ___UD__________H_G________________
> > 7fec7a601 112c01 1ff ________________TG________________
> > ^^^
> > 7fec7a800 113200 1 ___UD__________H_G________________
> > 7fec7a801 113201 1ff ________________TG________________
> > ^^^
> > OK
> >
> > Changelog:
> > - add hugetlb entry walker in mm/pagewalk.c
> > (the idea based on Kamezawa-san's patch)
> >
> Seems good.
>
> More info.
Thanks, I'll add this to patch description.
> - This patch modifies walk_page_range()'s hugepage walker.
> But the change only affects pagemap_read(), it's only caller of hugepage callback.
>
> - Before patch, hugetlb_entry() callback is called once per pgd. Then,
> hugtlb_entry() has to walk pgd's contents by itself.
> This caused BUG.
I think there is misunderstanding on this part.
I would add this instead:
- Without this patch, hugetlb_entry() callback is called once per vma,
that doesn't match the natural expectation from its name.
>
> - After patch, hugetlb_entry() callback is called once per hugepte entry.
> Then, callback will be much simpler.
>
>
...
> > +static int walk_hugetlb_range(struct vm_area_struct *vma,
> > + unsigned long addr, unsigned long end,
> > + struct mm_walk *walk)
> > +{
> > + struct hstate *h = hstate_vma(vma);
> > + unsigned long next;
> > + unsigned long hmask = huge_page_mask(h);
> > + pte_t *pte;
> > + int err = 0;
> > +
> > + do {
> > + next = hugetlb_entry_end(h, addr, end);
> > + pte = huge_pte_offset(walk->mm, addr & hmask);
> > + if (pte && walk->hugetlb_entry)
> > + err = walk->hugetlb_entry(pte, hmask, addr, next, walk);
> > + if (err)
> > + return err;
> > + } while (addr = next, addr != end);
> > +
> > + return err;
> > +}
> nitpick.
>
> seems nicer than mine but "return 0" is ok if you add "return err" in the loop.
>
OK. I fixed it.
Thanks,
Naoya Horiguchi
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>