Re: HUGEPAGE SIZE a boottime option

Jack Steiner
2004-02-24T04:05:59Z

On Mon, Feb 23, 2004 at 08:26:32AM -0800, Chen, Kenneth W wrote:
> sorry, missed this important hunk:
> # else
> # error Unsupported IA-64 HugeTLB Page Size!
> # endif
> +#define HPAGE_SHIFT hpage_shift
>
> Here is a work-in-progress patch that includes more comments we have.

Looks reasonable to me....

>
> (1) hugepagesz parameter should have min/max checked. Doesn't make
> sense to config huge page size smaller than PAGE_SIZE, or config huge
> page size larger than what page allocator allows (MAX_ORDER).
>
> (2) We can avoid patching vhpt handler and still allow dynamic sizing.
>
> (3) we remain unhappy with penalty hit on reload_context(). Region
> register 4 now has a dependency on loading variable hpage_shift, which
> could have worst case two/three hundred cycles. This variable is next
> to ia64_ctx (which is heavily used), but there is no guarantee that
> they sits in the same cache line. I've tried prefetch() with
> gcc-3.2.3, but it generates code that everyone can laugh at it.
>
> (4) If we have gone this far, it probably won't take that much more
> to make it runtime configurable!
>
>
> - Ken
>
>
> -----Original Message-----
> From: Jack Steiner [mailto:steiner@sgi.com]
> Sent: Sunday, February 22, 2004 3:08 PM
> To: Chen, Kenneth W
> Cc: linux-ia64@vger.kernel.org
> Subject: Re: HUGEPAGE SIZE a boottime option
>
>
> On Thu, Feb 19, 2004 at 08:00:04PM -0800, Chen, Kenneth W wrote:
> > It is not functionally complete though. alloc_fresh_huge_page(),
> > hugetlb_free_pgtables(), and update_and_free_page has #define
> > constant that indirectly from HPAGE_SHIFT.
> >
> > You might checked already, text replication works in this case?
> >
>
> (I posted this earlier. However, our mail server has been messed up & I dont think
> the mail got thru. Excuse the duplicate if the other mail ever makes it....)
>
>
> The patch passes preliminary testing.
>
> I dont see any issues with #define constants indirectly using HPAGE_SHIFT. HPAGE_SHIFT
> is now defined as:
> #define HPAGE_SHIFT hpage_shift
> and
> extern int hpage_shift;
> int hpage_shift=HPAGE_SHIFT_DEFAULT;
>
>
> Indirect references should work ok.
>
>
> > -----Original Message-----
> > From: linux-ia64-owner@vger.kernel.org
> > [mailto:linux-ia64-owner@vger.kernel.org]On Behalf Of Jack Steiner
> > Sent: Thursday, February 19, 2004 5:08 PM
> > To: linux-ia64@vger.kernel.org
> > Subject: HUGEPAGE SIZE a boottime option
> >
> >
> > Here is a preliminary version of a patch that makes the size of
> > HUGEPAGES a boottime option. Only ia64-specific files are changed (except
> > for the Documentation file).
> >
> > We have a number of customers using large pages. Unfortunately,
> > the "optimum" size of a large page is application & configuration
> > dependent. Rather that having each customer recompile to specify their
> > own HUGEPAGESIZE, this patch make the size a boottime option.
> >
> >
> > Does this patch look reasonable? If so, I will incorporate feedback,
> > finish testing it, update it to 2.6.3 & resubmit.

Content-Description: htlb_size.patch
> [-- octet_filter file type: "'diff' output text" --]
>
> [-- Statistics (lines words chars): 106 377 3429 /tmp/htlb_size.patch --]
>
> diff -Nur linux-2.6.3/arch/ia64/kernel/ivt.S linux-2.6.3.htlb/arch/ia64/kernel/ivt.S
> --- linux-2.6.3/arch/ia64/kernel/ivt.S 2004-02-17 19:57:16.000000000 -0800
> +++ linux-2.6.3.htlb/arch/ia64/kernel/ivt.S 2004-02-22 23:13:34.000000000 -0800
> @@ -118,10 +118,11 @@
> #ifdef CONFIG_HUGETLB_PAGE
> extr.u r26=r25,2,6
> ;;
> - cmp.eq p8,p0=HPAGE_SHIFT,r26
> + cmp.ne p8,p0=r18,r26
> + sub r27=r26,r18
> ;;
> (p8) dep r25=r18,r25,2,6
> -(p8) shr r22=r22,HPAGE_SHIFT-PAGE_SHIFT
> +(p8) shr r22=r22,r27
> #endif
> ;;
> cmp.eq p6,p7=5,r17 // is IFA pointing into to region 5?
> diff -Nur linux-2.6.3/arch/ia64/mm/hugetlbpage.c linux-2.6.3.htlb/arch/ia64/mm/hugetlbpage.c
> --- linux-2.6.3/arch/ia64/mm/hugetlbpage.c 2004-02-17 19:58:01.000000000 -0800
> +++ linux-2.6.3.htlb/arch/ia64/mm/hugetlbpage.c 2004-02-22 22:58:58.000000000 -0800
> @@ -23,6 +23,7 @@
> static long htlbpagemem;
> int htlbpage_max;
> static long htlbzone_pages;
> +unsigned int hpage_shift=HPAGE_SHIFT_DEFAULT;
>
> static struct list_head hugepage_freelists[MAX_NUMNODES];
> static spinlock_t htlbpage_lock = SPIN_LOCK_UNLOCKED;
> @@ -520,6 +521,30 @@
> }
> __setup("hugepages=", hugetlb_setup);
>
> +static int __init hugetlb_setup_sz(char *str)
> +{
> + u64 tr_pages;
> + unsigned long long size;
> +
> + if (ia64_pal_vm_page_size(&tr_pages, NULL) != 0)
> + /*
> + * shouldn't happen, but just in case.
> + */
> + tr_pages = 0x15557000UL;
> +
> + size = memparse(str, &str);
> + if (*str || (size & (size-1)) || !(tr_pages & size) ||
> + size <= PAGE_SIZE ||
> + size >= (1UL << PAGE_SHIFT << MAX_ORDER)) {
> + printk(KERN_WARNING "Invalid huge page size specified\n");
> + return 1;
> + }
> +
> + hpage_shift = __ffs(size);
> + return 1;
> +}
> +__setup("hugepagesz=", hugetlb_setup_sz);
> +
> static int __init hugetlb_init(void)
> {
> int i;
> diff -Nur linux-2.6.3/include/asm-ia64/page.h linux-2.6.3.htlb/include/asm-ia64/page.h
> --- linux-2.6.3/include/asm-ia64/page.h 2004-02-17 19:57:16.000000000 -0800
> +++ linux-2.6.3.htlb/include/asm-ia64/page.h 2004-02-22 17:26:18.000000000 -0800
> @@ -37,26 +37,26 @@
> #define RGN_MAP_LIMIT ((1UL << (4*PAGE_SHIFT - 12)) - PAGE_SIZE) /* per region addr limit */
>
> #ifdef CONFIG_HUGETLB_PAGE
> -
> # if defined(CONFIG_HUGETLB_PAGE_SIZE_4GB)
> -# define HPAGE_SHIFT 32
> +# define HPAGE_SHIFT_DEFAULT 32
> # elif defined(CONFIG_HUGETLB_PAGE_SIZE_1GB)
> -# define HPAGE_SHIFT 30
> +# define HPAGE_SHIFT_DEFAULT 30
> # elif defined(CONFIG_HUGETLB_PAGE_SIZE_256MB)
> -# define HPAGE_SHIFT 28
> +# define HPAGE_SHIFT_DEFAULT 28
> # elif defined(CONFIG_HUGETLB_PAGE_SIZE_64MB)
> -# define HPAGE_SHIFT 26
> +# define HPAGE_SHIFT_DEFAULT 26
> # elif defined(CONFIG_HUGETLB_PAGE_SIZE_16MB)
> -# define HPAGE_SHIFT 24
> +# define HPAGE_SHIFT_DEFAULT 24
> # elif defined(CONFIG_HUGETLB_PAGE_SIZE_4MB)
> -# define HPAGE_SHIFT 22
> +# define HPAGE_SHIFT_DEFAULT 22
> # elif defined(CONFIG_HUGETLB_PAGE_SIZE_1MB)
> -# define HPAGE_SHIFT 20
> +# define HPAGE_SHIFT_DEFAULT 20
> # elif defined(CONFIG_HUGETLB_PAGE_SIZE_256KB)
> -# define HPAGE_SHIFT 18
> +# define HPAGE_SHIFT_DEFAULT 18
> # else
> # error Unsupported IA-64 HugeTLB Page Size!
> # endif
> +#define HPAGE_SHIFT hpage_shift
>
> # define REGION_HPAGE (4UL) /* note: this is hardcoded in mmu_context.h:reload_context()!*/
> # define REGION_SHIFT 61
> @@ -140,6 +140,7 @@
> # define is_hugepage_only_range(addr, len) \
> (REGION_NUMBER(addr) = REGION_HPAGE && \
> REGION_NUMBER((addr)+(len)) = REGION_HPAGE)
> +extern unsigned int hpage_shift;
> #endif
>
> static __inline__ int

--
Thanks

Jack Steiner (steiner@sgi.com) 651-683-5302
Principal Engineer SGI - Silicon Graphics, Inc.