Re: [Libhugetlbfs-devel] Buglet in 16G page handling

David Gibson
2008-09-05T00:48:45Z

On Wed, Sep 03, 2008 at 05:19:27PM -0500, Jon Tollefson wrote:
> David Gibson wrote:
> > On Tue, Sep 02, 2008 at 12:12:27PM -0500, Jon Tollefson wrote:
> >
> >> David Gibson wrote:
> >>
> >>> When BenH and I were looking at the new code for handling 16G pages,
> >>> we noticed a small bug. It doesn't actually break anything user
> >>> visible, but it's certainly not the way things are supposed to be.
> >>> The 16G patches didn't update the huge_pte_offset() and
> >>> huge_pte_alloc() functions, which means that the hugepte tables for
> >>> 16G pages will be allocated much further down the page table tree than
> >>> they should be - allocating several levels of page table with a single
> >>> entry in them along the way.
> >>>
> >>> The patch below is supposed to fix this, cleaning up the existing
> >>> handling of 64k vs 16M pages while its at it. However, it needs some
> >>> testing.
> >>>
> >>> I've checked that it doesn't break existing 16M support, either with
> >>> 4k or 64k base pages. I haven't figured out how to test with 64k
> >>> pages yet, at least until the multisize support goes into
> >>> libhugetlbfs. For 16G pages, I just don't have access to a machine
> >>> with enough memory to test. Jon, presumably you must have found such
> >>> a machine when you did the 16G page support in the first place. Do
> >>> you still have access, and can you test this patch?
> >>>
> >>>
> >> I do have access to a machine to test it. I applied the patch to -rc4
> >> and used a pseries_defconfig. I boot with
> >> default_hugepagesz=16G... in order to test huge page sizes other then
> >> 16M at this point.
> >>
> >> Running the libhugetlbfs test suite it gets as far as Readback (64):
> >> PASS
> >> before it hits the following program check.
> >>
> >
> > Ah, yes, oops, forgot to fix up the pagetable freeing path in line
> > with the other changes. Try the revised version below.
> >
> I have run through the tests twice now with this new patch using a 4k
> base page size(and 16G huge page size) and there are no program checks
> or spin lock issues. So looking good.
>
> I will run it next a couple of times with 64K base pages.

Ok, and I've now run it with 64k hugepage size, so assuming this last
test of yours goes ok, I'll push the patch out.

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!