On Thu, 2 Mar 2006, Chen, Kenneth W wrote: > Hugh Dickins wrote on Thursday, March 02, 2006 10:53 AM > > > Whereas there's still a real ia64 get_unmapped_area bug to be fixed, > > arising from the same confusion, that is_hugepage_only_range needs > > to mean overlaps_hugepage_only_range (as on PowerPC) rather than > > within_hugepage_only_range (as on IA64). Is Ken fixing that one? > > Yes, I'm fixing it. See patch below. Great, thanks. The second part, using REGION_NUMBER instead of is_hugepage_only_range in the ia64 hugetlb_free_pgd_range, looks nice. But the first part, || instead of && in is_hugepage_only_range, looks insufficient: the start and end of the range might each fall in a non-huge region, but the range still cross a huge region. Ah, does RGN_HPAGE nestle up against the TASK_SIZE roof, so any range already tested against TASK_SIZE (as get_unmapped_area has) cannot cross RGN_HPAGE? If so, perhaps it deserves a comment there. And if that is so, and can be relied upon, is_hugepage_only_range need only be testing REGION_NUMBER(addr+len-1) - but it does seem fragile. Hugh > > [patch] ia64: fix is_hugepage_only_range() definition to be overlaps > instead of within architectural restricted hugetlb address > range. Fix all affected usages. > > Signed-off-by: Ken Chen <kenneth.w.chen@xxxxxxxxx> > > --- ./include/asm-ia64/page.h.orig 2006-03-02 12:16:00.636688455 -0800 > +++ ./include/asm-ia64/page.h 2006-03-02 12:23:30.151331386 -0800 > @@ -147,7 +147,7 @@ typedef union ia64_va { > | (REGION_OFFSET(x) >> (HPAGE_SHIFT-PAGE_SHIFT))) > # define HUGETLB_PAGE_ORDER (HPAGE_SHIFT - PAGE_SHIFT) > # define is_hugepage_only_range(mm, addr, len) \ > - (REGION_NUMBER(addr) == RGN_HPAGE && \ > + (REGION_NUMBER(addr) == RGN_HPAGE || \ > REGION_NUMBER((addr)+(len)-1) == RGN_HPAGE) > extern unsigned int hpage_shift; > #endif > --- ./arch/ia64/mm/hugetlbpage.c.orig 2006-03-02 12:31:12.020466353 -0800 > +++ ./arch/ia64/mm/hugetlbpage.c 2006-03-02 12:31:02.944294589 -0800 > @@ -112,8 +112,7 @@ void hugetlb_free_pgd_range(struct mmu_g > unsigned long floor, unsigned long ceiling) > { > /* > - * This is called only when is_hugepage_only_range(addr,), > - * and it follows that is_hugepage_only_range(end,) also. > + * This is called to free hugetlb page tables. > * > * The offset of these addresses from the base of the hugetlb > * region must be scaled down by HPAGE_SIZE/PAGE_SIZE so that > @@ -125,9 +124,9 @@ void hugetlb_free_pgd_range(struct mmu_g > > addr = htlbpage_to_page(addr); > end = htlbpage_to_page(end); > - if (is_hugepage_only_range(tlb->mm, floor, HPAGE_SIZE)) > + if (REGION_NUMBER(floor) == RGN_HPAGE) > floor = htlbpage_to_page(floor); > - if (is_hugepage_only_range(tlb->mm, ceiling, HPAGE_SIZE)) > + if (REGION_NUMBER(ceiling) == RGN_HPAGE) > ceiling = htlbpage_to_page(ceiling); > > free_pgd_range(tlb, addr, end, floor, ceiling); - : send the line "unsubscribe linux-ia64" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
![]() |