On 03.03.2007 [18:02:07 +1100], David Gibson wrote:
> On Fri, Mar 02, 2007 at 03:46:07PM -0800, Nishanth Aravamudan wrote:
> > On 02.03.2007 [18:24:26 +1100], David Gibson wrote:
> > > On Wed, Feb 28, 2007 at 08:22:36AM -0800, Nishanth Aravamudan wrote:
> > > > On 27.02.2007 [10:47:31 +1100], David Gibson wrote:
> > > > > On Mon, Feb 26, 2007 at 03:21:26PM -0800, Nishanth Aravamudan wrote:
> > > > > > Author: Nishanth Aravamudan <[EMAIL PROTECTED]>
> > > > > > Date:   Mon Feb 26 15:17:54 2007 -0800
> > > > > > 
> > > > > > hugeutils: add address parameter to hugetlbfs_vaddr_granularity
> > > > > > 
> > > > > > On ppc64, the actual granularity of hugepage requests depends on the
> > > > > > address in question. Specify the three cases (0-4G, 4G-1TB, 1TB+) 
> > > > > > in the
> > > > > > function. This is useful for the partial_remap code, as it would
> > > > > > (effectively) never run on ppc64 otherwise (would need a 1TB or 
> > > > > > larger
> > > > > > segment, without relinking).
> > > > > 
> > > > > I've thought about doing something like this before.  However, the
> > > > > semantics as they stand are a little bit fuzzy.  What I'd prefer, if
> > > > > you want to get finer-grained information about the page-size regions
> > > > > is a pair of functions to find the end and beginning of a page-size
> > > > > slice, given an address.  So:
> > > > > 
> > > > > void *hugetlbfs_slice_end(void *addr);
> > > > > void *hugetlbfs_slice_start(void *addr);
> > > > > 
> > > > > The "slice" terminology is something BenH has started using for these
> > > > > address space regionsin some recent kernel patches.
> > > > 
> > > > I'm willing to believe I'm doing something incredibly stupid, but here's
> > > > my patch. I had to use a bunch of macros, as I was getting a headache
> > > > from all the casts and the constants. How do I make this cleaner? Or do
> > > > I give up on this?
> > > 
> > > Well, for starters, you want to get rid of
> > > hugetlbfs_vaddr_granularity() entirely.  
> > 
> > <snip>
> > 
> > > Except.. I think hugetlbfs_slice_end() will actually need to return
> > > the end of the slice, not the beginning of the next one as it does
> > > now.  I don't like that semantic on the whole, but without it, for
> > > 32-bit programs hugetlbfs_slice_end() will return 0 for sufficiently
> > > high addresses.  I'd prefer to reserve the case where
> > > hugetlbfs_slice_end(adde) < addr to mean that the given address is
> > > already outside the potentially valid range for hugepage addresses (so
> > > if we ever support ia64, slice_start() and slice_end() end would
> > > return the constant addresses marking the ends of their fixed hugepage
> > > region).
> > 
> > Thanks for the review David. I'm not entirely sure about the semantics
> > still. I got what you were saying, but wasn't sure of the best way to
> > implement it.
> > 
> > So does something like the following look better?

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..7559904 100644
--- a/hugeutils.c
+++ b/hugeutils.c
@@ -114,14 +114,36 @@ long gethugepagesize(void)
        return hpage_size;
 }
 
-long hugetlbfs_vaddr_granularity(void)
-{
+/*
+ * 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__)
-       return (1L << 40);
+       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..ba6158a 100644
--- 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)
+
+#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