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. Jon > Index: working-2.6/arch/powerpc/mm/hugetlbpage.c > =================================================================== > --- working-2.6.orig/arch/powerpc/mm/hugetlbpage.c 2008-09-02 > 11:50:12.000000000 +1000 > +++ working-2.6/arch/powerpc/mm/hugetlbpage.c 2008-09-03 10:10:54.000000000 > +1000 > @@ -128,29 +128,37 @@ static int __hugepte_alloc(struct mm_str > return 0; > } > > -/* Base page size affects how we walk hugetlb page tables */ > -#ifdef CONFIG_PPC_64K_PAGES > -#define hpmd_offset(pud, addr, h) pmd_offset(pud, addr) > -#define hpmd_alloc(mm, pud, addr, h) pmd_alloc(mm, pud, addr) > -#else > -static inline > -pmd_t *hpmd_offset(pud_t *pud, unsigned long addr, struct hstate *hstate) > + > +static pud_t *hpud_offset(pgd_t *pgd, unsigned long addr, struct hstate > *hstate) > +{ > + if (huge_page_shift(hstate) < PUD_SHIFT) > + return pud_offset(pgd, addr); > + else > + return (pud_t *) pgd; > +} > +static pud_t *hpud_alloc(struct mm_struct *mm, pgd_t *pgd, unsigned long > addr, > + struct hstate *hstate) > { > - if (huge_page_shift(hstate) == PAGE_SHIFT_64K) > + if (huge_page_shift(hstate) < PUD_SHIFT) > + return pud_alloc(mm, pgd, addr); > + else > + return (pud_t *) pgd; > +} > +static pmd_t *hpmd_offset(pud_t *pud, unsigned long addr, struct hstate > *hstate) > +{ > + if (huge_page_shift(hstate) < PMD_SHIFT) > return pmd_offset(pud, addr); > else > return (pmd_t *) pud; > } > -static inline > -pmd_t *hpmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long addr, > - struct hstate *hstate) > +static pmd_t *hpmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long > addr, > + struct hstate *hstate) > { > - if (huge_page_shift(hstate) == PAGE_SHIFT_64K) > + if (huge_page_shift(hstate) < PMD_SHIFT) > return pmd_alloc(mm, pud, addr); > else > return (pmd_t *) pud; > } > -#endif > > /* Build list of addresses of gigantic pages. This function is used in early > * boot before the buddy or bootmem allocator is setup. > @@ -204,7 +212,7 @@ pte_t *huge_pte_offset(struct mm_struct > > pg = pgd_offset(mm, addr); > if (!pgd_none(*pg)) { > - pu = pud_offset(pg, addr); > + pu = hpud_offset(pg, addr, hstate); > if (!pud_none(*pu)) { > pm = hpmd_offset(pu, addr, hstate); > if (!pmd_none(*pm)) > @@ -233,7 +241,7 @@ pte_t *huge_pte_alloc(struct mm_struct * > addr &= hstate->mask; > > pg = pgd_offset(mm, addr); > - pu = pud_alloc(mm, pg, addr); > + pu = hpud_alloc(mm, pg, addr, hstate); > > if (pu) { > pm = hpmd_alloc(mm, pu, addr, hstate); > @@ -316,13 +324,7 @@ static void hugetlb_free_pud_range(struc > pud = pud_offset(pgd, addr); > do { > next = pud_addr_end(addr, end); > -#ifdef CONFIG_PPC_64K_PAGES > - if (pud_none_or_clear_bad(pud)) > - continue; > - hugetlb_free_pmd_range(tlb, pud, addr, next, floor, ceiling, > - psize); > -#else > - if (shift == PAGE_SHIFT_64K) { > + if (shift < PMD_SHIFT) { > if (pud_none_or_clear_bad(pud)) > continue; > hugetlb_free_pmd_range(tlb, pud, addr, next, floor, > @@ -332,7 +334,6 @@ static void hugetlb_free_pud_range(struc > continue; > free_hugepte_range(tlb, (hugepd_t *)pud, psize); > } > -#endif > } while (pud++, addr = next, addr != end); > > start &= PGDIR_MASK; > @@ -422,9 +423,15 @@ void hugetlb_free_pgd_range(struct mmu_g > psize = get_slice_psize(tlb->mm, addr); > BUG_ON(!mmu_huge_psizes[psize]); > next = pgd_addr_end(addr, end); > - if (pgd_none_or_clear_bad(pgd)) > - continue; > - hugetlb_free_pud_range(tlb, pgd, addr, next, floor, ceiling); > + if (mmu_psize_to_shift(psize) < PUD_SHIFT) { > + if (pgd_none_or_clear_bad(pgd)) > + continue; > + hugetlb_free_pud_range(tlb, pgd, addr, next, floor, > ceiling); > + } else { > + if (pgd_none(*pgd)) > + continue; > + free_hugepte_range(tlb, (hugepd_t *)pgd, psize); > + } > } while (pgd++, addr = next, addr != end); > } > > > > _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev