RE: hugepage patches

["Seth, Rohit" <rohit.seth@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

> OK, but it needs some changes.
> 
> - is_valid_hugepage_range() will not compile.  `addrp' vs `addr'
> 
> - We should not pass in a flag variable which alters a 
> function's behaviour
>   in this manner.  Especially when it has the wonderful name 
> "flag", and no
>   supporting commentary!
> 
>   Please split this into two separate (and documented) functions.


Attached is the updated patch based on your comments.  

> 
> - A name like "is_valid_hugepage_range" implies that this function is
>   purely a predicate.  Yet it is capable of altering part of 
> the caller's
>   environment.  Can we have a more appropriate name?
> 
> - I've been trying to keep ia64/sparc64/x86_64 as uptodate as I can
>   throughout this.  I think we can safely copy the ia32 
> implementation over
>   into there as well, can't we?

For ia64, there is a separate kernel patch that David Mosberger
maintains.  Linus's tree won't work as is on ia64. Not sure about
x86_64/sparc64.

> 
>   If there's any doubt then probably it's best to just leave 
> the symbol
>   undefined, let the arch maintainers curse us ;)

> 
> Are you working against Linus's current tree?  A lot has 
> changed in there. 
> I'd like to hear if hugetlbfs is working correctly in a 
> non-ia32 kernel.

Yeah, I am working on Linus's 2.5.59 tree. Will download your mm9 to get
my tree updated.  Is there any other patch that you want me to apply
before sending you any more updates.

As far as non-ia32 kernel is concerned, hugetlbfs on ia64 should be
working fine. Though I've not yet tried the 2.5.59 on ia64. 2.5.59 ia64
patch that David maintains has the same level of hugetlb support as i386
tree. 


> 

[-- Attachment #2: patch.750 --]
[-- Type: application/octet-stream, Size: 2635 bytes --]

--- mm/mmap.c.750	Fri Feb  7 18:50:27 2003
+++ mm/mmap.c	Fri Feb  7 18:50:55 2003
@@ -682,7 +682,7 @@
 			return -ENOMEM;
 		if (addr & ~PAGE_MASK)
 			return -EINVAL;
-		if (is_file_hugepages(file) && (ret = is_valid_hugepage_range(&addr, len, 1)))
+		if (is_file_hugepages(file) && (ret = is_align_hugepage_range(addr, len)))
 			return ret;
 		return addr;
 	}
--- include/linux/hugetlb.h.750	Fri Feb  7 18:55:37 2003
+++ include/linux/hugetlb.h	Fri Feb  7 18:49:24 2003
@@ -21,7 +21,8 @@
 void hugetlb_release_key(struct hugetlb_key *);
 int hugetlb_report_meminfo(char *);
 int is_hugepage_mem_enough(size_t);
-unsigned long is_valid_hugepage_range(unsigned long *, unsigned long, int);
+unsigned long chk_align_and_fix_addr(unsigned long *, unsigned long);
+unsigned long is_align_hugepage_range(unsigned long, unsigned long);
 
 extern int htlbpage_max;
 
@@ -39,7 +40,7 @@
 #define huge_page_release(page)			BUG()
 #define is_hugepage_mem_enough(size)		0
 #define hugetlb_report_meminfo(buf)		0
-#define is_valid_hugepage_range(addr, len, flg)		0
+#define is_align_hugepage_range(addr, len)	0
 
 #endif /* !CONFIG_HUGETLB_PAGE */
 
--- arch/i386/mm/hugetlbpage.c.750	Fri Feb  7 18:40:54 2003
+++ arch/i386/mm/hugetlbpage.c	Fri Feb  7 18:59:17 2003
@@ -88,17 +88,24 @@
 	set_pte(page_table, entry);
 }
 
-unsigned long is_valid_hugepage_range(unsigned long *addrp, unsigned long len, int flag)
+/* This function checks for proper alignment for len.  It updates the input addrp parameter so that
+ * it points to valid(and aligned) hugepage address range (For i386 it is just proper alignment).
+ */
+unsigned long chk_align_and_fix_addr(unsigned long *addrp, unsigned long len)
 {
 	if (len & ~HPAGE_MASK)
 		return -EINVAL;
-	if (flag) {
-		if (*addr & ~HPAGE_MASK)
-			return -EINVAL;
-		return 0;
-	}
-	if (len > TASK_SIZE)
-		return -ENOMEM;
+	*addrp = ALIGN(*addrp, HPAGE_SIZE);
+	return 0;
+}
+/* This function checks for proper alignement of input addr and len parameters.
+ */
+unsigned long is_align_hugepage_range(unsigned long addr, unsigned long len)
+{
+	if (len & ~HPAGE_MASK)
+		return -EINVAL;
+	if (addr & ~HPAGE_MASK)
+		return -EINVAL;
 	return 0;
 }
 
--- fs/hugetlbfs/inode.c.750	Fri Feb  7 18:38:31 2003
+++ fs/hugetlbfs/inode.c	Fri Feb  7 18:46:04 2003
@@ -89,11 +89,10 @@
 	struct vm_area_struct *vma;
 	unsigned long ret = 0;
 
-	if (ret = is_valid_hugepage_range(&addr, len, 0))
+	if (ret = chk_align_and_fix_addr(&addr, len))
 		return ret;
 
 	if (addr) {
-		addr = ALIGN(addr, HPAGE_SIZE);
 		vma = find_vma(mm, addr);
 		if (TASK_SIZE - len >= addr &&
 		    (!vma || addr + len <= vma->vm_start))