On 04.03.2007 [09:55:09 +1100], David Gibson wrote:
> On Sat, Mar 03, 2007 at 01:53:11PM -0800, Nishanth Aravamudan wrote:
> [snip]
> > hugeutils: remove hugetlbfs_vaddr_granularity in favor of slice functions
> > 
> > hugetlbfs_vaddr_granularity() currently does not take a parameter, even
> > though the granularity at which hugepages can be used varies by address
> > on ppc64 (and perhaps other archs). Taking a queue from Ben
> > Herrenschmidt's kernel slice patches for Cell add similar functions and
> > macros for hugetlbfs_slice_start() and hugetlbfs_slice_end() which,
> > given an unsigned long address, will indicate where the enclosing slice
> > starts and ends, inclusively (that is the start and end are part of the
> > slice). This has very clean and useful semantics compared to the
> > granularity() function. This is also useful for the partial_remap code,
> > as it would (effectively) never run on ppc64 otherwise (would need a 1TB
> > or larger segment, without relinking, which is unlikely). Thanks to
> > David Gibson for several rounds of review.
> 
> Looks good, save for one thing now.
> 
> [...]
> > --- a/libhugetlbfs_internal.h
> > +++ b/libhugetlbfs_internal.h
> > @@ -26,7 +26,16 @@
> >  #define stringify_1(x)     #x
> >  #define stringify(x)       stringify_1(x)
> >  
> > -#define ALIGN(x, a)        (((x) + (a) - 1) & ~((a) - 1))
> > +#define SLICE_LOW_SHIFT            28
> > +#define SLICE_HIGH_SHIFT   40
> > +
> > +#define SLICE_LOW_TOP              (0x100000000UL)
> > +#define SLICE_LOW_SIZE             (1UL << SLICE_LOW_SHIFT)
> > +#define SLICE_HIGH_SIZE            (1UL << SLICE_HIGH_SHIFT)
> 
> The SLICE_* constants don't belong here.  They should go right next to
> the slice functions, preferably within one of the #ifdefs, since
> they're powerpc specific.

Good point, does the following get your ACK?

hugeutils: remove hugetlbfs_vaddr_granularity in favor of slice functions

hugetlbfs_vaddr_granularity() currently does not take a parameter, even
though the granularity at which hugepages can be used varies by address
on ppc64 (and perhaps other archs). Taking a queue from Ben
Herrenschmidt's kernel slice patches for Cell add similar functions and
macros for hugetlbfs_slice_start() and hugetlbfs_slice_end() which,
given an unsigned long address, will indicate where the enclosing slice
starts and ends, inclusively (that is the start and end are part of the
slice). This has very clean and useful semantics compared to the
granularity() function. This is also useful for the partial_remap code,
as it would (effectively) never run on ppc64 otherwise (would need a 1TB
or larger segment, without relinking, which is unlikely). Thanks to
David Gibson for several rounds of review.

Signed-off-by: Nishanth Aravamudan <[EMAIL PROTECTED]>

diff --git a/hugetlbfs.h b/hugetlbfs.h
index 966e57c..be5a352 100644
--- a/hugetlbfs.h
+++ b/hugetlbfs.h
@@ -22,7 +22,8 @@
 #define HUGETLBFS_MAGIC        0x958458f6
 
 long gethugepagesize(void);
-long hugetlbfs_vaddr_granularity(void);
+unsigned long hugetlbfs_slice_start(unsigned long);
+unsigned long hugetlbfs_slice_end(unsigned long);
 int hugetlbfs_test_path(const char *mount);
 const char *hugetlbfs_find_path(void);
 int hugetlbfs_unlinked_fd(void);
diff --git a/hugeutils.c b/hugeutils.c
index 9af92b9..39d19e2 100644
--- a/hugeutils.c
+++ b/hugeutils.c
@@ -114,14 +114,45 @@ long gethugepagesize(void)
        return hpage_size;
 }
 
-long hugetlbfs_vaddr_granularity(void)
-{
 #if defined(__powerpc64__)
-       return (1L << 40);
+#define SLICE_LOW_SHIFT                28
+#define SLICE_HIGH_SHIFT       40
+
+#define SLICE_LOW_TOP          (0x100000000UL)
+#define SLICE_LOW_SIZE         (1UL << SLICE_LOW_SHIFT)
+#define SLICE_HIGH_SIZE                (1UL << SLICE_HIGH_SHIFT)
+#endif
+
+/*
+ * Return the address of the start and end of the hugetlb slice
+ * containing @addr. A slice is a range of addresses, start inclusive
+ * and end exclusive.
+ */
+unsigned long hugetlbfs_slice_start(unsigned long addr) {
+#if defined(__powerpc64__)
+       if (addr < SLICE_LOW_TOP)
+               return ALIGN_DOWN(addr, SLICE_LOW_SIZE);
+       else if (addr < SLICE_HIGH_SIZE)
+               return SLICE_LOW_TOP;
+       else
+               return ALIGN_DOWN(addr, SLICE_HIGH_SIZE);
+#elif defined(__powerpc__)
+       return ALIGN_DOWN(addr, SLICE_LOW_SIZE);
+#else
+       return ALIGN_DOWN(addr, gethugepagesize());
+#endif
+}
+
+unsigned long hugetlbfs_slice_end(unsigned long addr) {
+#if defined(__powerpc64__)
+       if (addr < SLICE_LOW_TOP)
+               return ALIGN_UP(addr, SLICE_LOW_SIZE) - 1;
+       else
+               return ALIGN_UP(addr, SLICE_HIGH_SIZE) - 1;
 #elif defined(__powerpc__)
-       return (1L << 28);
+       return ALIGN_UP(addr, SLICE_LOW_SIZE) - 1;
 #else
-       return gethugepagesize();
+       return ALIGN_UP(addr, gethugepagesize()) - 1;
 #endif
 }
 
diff --git a/libhugetlbfs_internal.h b/libhugetlbfs_internal.h
index 40da66a..ae4cb4c 100644
--- a/libhugetlbfs_internal.h
+++ b/libhugetlbfs_internal.h
@@ -26,7 +26,9 @@
 #define stringify_1(x) #x
 #define stringify(x)   stringify_1(x)
 
-#define ALIGN(x, a)    (((x) + (a) - 1) & ~((a) - 1))
+#define ALIGN_UP(x, a)         (((x) + (a) - 1) & ~((a) - 1))
+#define ALIGN_DOWN(x, a)       ((x) & ~((a) - 1))
+#define ALIGN(x,a)             ALIGN_UP(x,a)
 
 extern int __hugetlbfs_verbose;
 
diff --git a/morecore.c b/morecore.c
index 25c2d27..fca4ff0 100644
--- a/morecore.c
+++ b/morecore.c
@@ -166,7 +166,7 @@ static void __attribute__((constructor)) 
setup_morecore(void)
                }
        } else {
                heapaddr = (unsigned long)sbrk(0);
-               heapaddr = ALIGN(heapaddr, hugetlbfs_vaddr_granularity());
+               heapaddr = hugetlbfs_slice_end(heapaddr) + 1;
        }
 
        DEBUG("setup_morecore(): heapaddr = 0x%lx\n", heapaddr);

-- 
Nishanth Aravamudan <[EMAIL PROTECTED]>
IBM Linux Technology Center

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Libhugetlbfs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libhugetlbfs-devel

Reply via email to