Skip to contentSkip to conversation

Demand faulting for hugetlb

PATCH In review Adam Litke 4 participants 10 messages Oct 11, 2005 – Oct 14, 2005

Subsystems (3)
lore
Share & tools
lore

Adam Litke

author
#0 lore raw

cover Demand faulting for hugetlb

Ok, here's the next iteration of these patches. I think I've handled the truncate() case by comparing the hugetlbfs inode's i_size with the mapping offset of the requested page to make sure it hasn't been truncated. Can anyone confirm or deny that I have the locking correct for this? The other patches are still unchanged. Andrew: Did Andi Kleen's explanation of huge_pages_needed() satisfy?

Signature · 8 lines
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#1 lore raw

PATCH 1/3 hugetlb: Remove get_user_pages optimization

Version 2 (Mon, 3 Oct 2005)
* Removed follow_hugetlb_page since get_user_pages() was the only caller.

Initial Post (Thu, 18 Aug 2005)

In preparation for hugetlb demand faulting, remove this get_user_pages() optimization. Since huge pages will no longer be prefaulted, we can't assume that the huge ptes are established and hence, calling follow_hugetlb_page() is not valid.

With the follow_hugetlb_page() call removed, the normal code path will be triggered. follow_page() will either use follow_huge_addr() or follow_huge_pmd() to check for a previously faulted "page" to return. When this fails (ie. with demand faults), __handle_mm_fault() gets called which invokes the hugetlb_fault() handler to instantiate the huge page.

This patch doesn't make a lot of sense by itself, but I've broken it out to facilitate discussion on this specific element of the demand fault changes. While coding this up, I referenced previous discussion on this topic starting at http://lkml.org/lkml/2004/4/13/176 , which contains more opinions about the correctness of this approach.

---
 hugetlb.c |   54 ------------------------------------------------------
 memory.c  |    5 -----
 2 files changed, 59 deletions(-)
mm/hugetlb.c +0 -54
diff -upN reference/mm/hugetlb.c current/mm/hugetlb.c
@@ -392,57 +392,3 @@ out:
 	spin_unlock(&mm->page_table_lock);
 	return ret;
 }
-
-int follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
-			struct page **pages, struct vm_area_struct **vmas,
-			unsigned long *position, int *length, int i)
-{
-	unsigned long vpfn, vaddr = *position;
-	int remainder = *length;
-
-	BUG_ON(!is_vm_hugetlb_page(vma));
-
-	vpfn = vaddr/PAGE_SIZE;
-	spin_lock(&mm->page_table_lock);
-	while (vaddr < vma->vm_end && remainder) {
-
-		if (pages) {
-			pte_t *pte;
-			struct page *page;
-
-			/* Some archs (sparc64, sh*) have multiple
-			 * pte_ts to each hugepage.  We have to make
-			 * sure we get the first, for the page
-			 * indexing below to work. */
-			pte = huge_pte_offset(mm, vaddr & HPAGE_MASK);
-
-			/* the hugetlb file might have been truncated */
-			if (!pte || pte_none(*pte)) {
-				remainder = 0;
-				if (!i)
-					i = -EFAULT;
-				break;
-			}
-
-			page = &pte_page(*pte)[vpfn % (HPAGE_SIZE/PAGE_SIZE)];
-
-			WARN_ON(!PageCompound(page));
-
-			get_page(page);
-			pages[i] = page;
-		}
-
-		if (vmas)
-			vmas[i] = vma;
-
-		vaddr += PAGE_SIZE;
-		++vpfn;
-		--remainder;
-		++i;
-	}
-	spin_unlock(&mm->page_table_lock);
-	*length = remainder;
-	*position = vaddr;
-
-	return i;
-}
mm/memory.c +0 -5
diff -upN reference/mm/memory.c current/mm/memory.c
@@ -971,11 +971,6 @@ int get_user_pages(struct task_struct *t
 				|| !(flags & vma->vm_flags))
 			return i ? : -EFAULT;
 
-		if (is_vm_hugetlb_page(vma)) {
-			i = follow_hugetlb_page(mm, vma, pages, vmas,
-						&start, &len, i);
-			continue;
-		}
 		spin_lock(&mm->page_table_lock);
 		do {
 			int write_access = write;
Signature · 8 lines
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#3 lore raw

PATCH 3/3 hugetlb: Simple overcommit check

Quoted 6 lines · Andrew Morton
Quoted 3 lines · Adam Litke

Andrew: Did Andi
Kleen's explanation of huge_pages_needed() satisfy?

Spose so. I trust that it's adequately commented in this version..

Just to be sure, added a comment block at the top of huge_pages_needed().

Initial Post (Thu, 18 Aug 2005)

Basic overcommit checking for hugetlb_file_map() based on an implementation used with demand faulting in SLES9.

Since we're not prefaulting the pages at mmap time, some extra accounting is needed. This patch implements a basic sanity check to ensure that the number of huge pages required to satisfy the mmap are currently available. Of course this method doesn't guarantee that the pages will be available at fault time, but I think it is a good start on doing proper accounting and solves 90% of the overcommit problems I see in practice.

Huge page shared memory segments are simpler and still maintain their commit on shmget semantics.

---
 inode.c |   73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 63 insertions(+), 10 deletions(-)
fs/hugetlbfs/inode.c +63 -10
diff -upN reference/fs/hugetlbfs/inode.c current/fs/hugetlbfs/inode.c
@@ -45,9 +45,67 @@ static struct backing_dev_info hugetlbfs
 
 int sysctl_hugetlb_shm_group;
 
+static void huge_pagevec_release(struct pagevec *pvec)
+{
+	int i;
+
+	for (i = 0; i < pagevec_count(pvec); ++i)
+		put_page(pvec->pages[i]);
+
+	pagevec_reinit(pvec);
+}
+
+/*
+ * huge_pages_needed tries to determine the number of new huge pages that
+ * will be required to fully populate this VMA.  This will be equal to
+ * the size of the VMA in huge pages minus the number of huge pages 
+ * (covered by this VMA) that are found in the page cache.
+ *
+ * Result is in bytes to be compatible with is_hugepage_mem_enough()
+ */
+unsigned long
+huge_pages_needed(struct address_space *mapping, struct vm_area_struct *vma)
+{
+	int i;
+	struct pagevec pvec;
+	unsigned long start = vma->vm_start;
+	unsigned long end = vma->vm_end;
+	unsigned long hugepages = (end - start) >> HPAGE_SHIFT;
+	pgoff_t next = vma->vm_pgoff;
+	pgoff_t endpg = next + ((end - start) >> PAGE_SHIFT);
+	struct inode *inode = vma->vm_file->f_dentry->d_inode;
+
+	/*
+	 * Shared memory segments are accounted for at shget time,
+	 * not at shmat (when the mapping is actually created) so 
+	 * check here if the memory has already been accounted for.
+	 */
+	if (inode->i_blocks != 0)
+		return 0;
+
+	pagevec_init(&pvec, 0);
+	while (next < endpg) {
+		if (!pagevec_lookup(&pvec, mapping, next, PAGEVEC_SIZE))
+			break;
+		for (i = 0; i < pagevec_count(&pvec); i++) {
+			struct page *page = pvec.pages[i];
+			if (page->index > next)
+				next = page->index;
+			if (page->index >= endpg)
+				break;
+			next++;
+			hugepages--;
+		}
+		huge_pagevec_release(&pvec);
+	}
+	return hugepages << HPAGE_SHIFT;
+}
+
 static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct inode *inode = file->f_dentry->d_inode;
+	struct address_space *mapping = inode->i_mapping;
+	unsigned long bytes;
 	loff_t len, vma_len;
 	int ret;
 
@@ -66,6 +124,10 @@ static int hugetlbfs_file_mmap(struct fi
 	if (vma->vm_end - vma->vm_start < HPAGE_SIZE)
 		return -EINVAL;
 
+	bytes = huge_pages_needed(mapping, vma);
+	if (!is_hugepage_mem_enough(bytes))
+		return -ENOMEM;
+
 	vma_len = (loff_t)(vma->vm_end - vma->vm_start);
 
 	down(&inode->i_sem);
@@ -167,16 +229,6 @@ static int hugetlbfs_commit_write(struct
 	return -EINVAL;
 }
 
-static void huge_pagevec_release(struct pagevec *pvec)
-{
-	int i;
-
-	for (i = 0; i < pagevec_count(pvec); ++i)
-		put_page(pvec->pages[i]);
-
-	pagevec_reinit(pvec);
-}
-
 static void truncate_huge_page(struct page *page)
 {
 	clear_page_dirty(page);
@@ -792,6 +844,7 @@ struct file *hugetlb_zero_setup(size_t s
 	d_instantiate(dentry, inode);
 	inode->i_size = size;
 	inode->i_nlink = 0;
+	inode->i_blocks = 1;
 	file->f_vfsmnt = mntget(hugetlbfs_vfsmount);
 	file->f_dentry = dentry;
 	file->f_mapping = inode->i_mapping;
Signature · 8 lines
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#4 lore raw

PATCH 2/3 hugetlb: Demand fault handler

In reply to Adam Litke

Version 5 (Tue, 11 Oct 2005)
Deal with hugetlbfs file truncation in find_get_huge_page()
Version 4 (Mon, 03 Oct 2005)
Make find_get_huge_page bale properly when add_to_page_cache fails
due to OOM conditions
Version 3 (Thu, 08 Sep 2005)
Organized logic in hugetlb_pte_fault() by breaking out
find_get_page/alloc_huge_page logic into separate function
Removed a few more paranoid checks ( Thanks )
Fixed tlb flushing in a race case ( Yanmin Zhang )

Version 2 (Wed, 17 Aug 2005)
Removed spurious WARN_ON()
Patches added earlier in the series (now in mainline):
Check for p?d_none() in arch/i386/mm/hugetlbpage.c:huge_pte_offset() Move i386 stale pte check into huge_pte_alloc()

Initial Post (Fri, 05 Aug 2005)

Below is a patch to implement demand faulting for huge pages. The main motivation for changing from prefaulting to demand faulting is so that huge page memory areas can be allocated according to NUMA policy.

Thanks to consolidated hugetlb code, switching the behavior requires changing only one fault handler. The bulk of the patch just moves the logic from hugelb_prefault() to hugetlb_pte_fault() and find_get_huge_page().

---
 fs/hugetlbfs/inode.c    |    6 --
 include/linux/hugetlb.h |    2 
 mm/hugetlb.c            |  139 ++++++++++++++++++++++++++++++++----------------
 mm/memory.c             |    2 
 4 files changed, 98 insertions(+), 51 deletions(-)
fs/hugetlbfs/inode.c +1 -5
diff -upN reference/fs/hugetlbfs/inode.c current/fs/hugetlbfs/inode.c
@@ -48,7 +48,6 @@ int sysctl_hugetlb_shm_group;
 static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct inode *inode = file->f_dentry->d_inode;
-	struct address_space *mapping = inode->i_mapping;
 	loff_t len, vma_len;
 	int ret;
 
@@ -79,10 +78,7 @@ static int hugetlbfs_file_mmap(struct fi
 	if (!(vma->vm_flags & VM_WRITE) && len > inode->i_size)
 		goto out;
 
-	ret = hugetlb_prefault(mapping, vma);
-	if (ret)
-		goto out;
-
+	ret = 0;
 	if (inode->i_size < len)
 		inode->i_size = len;
 out:
include/linux/hugetlb.h +2 -0
diff -upN reference/include/linux/hugetlb.h current/include/linux/hugetlb.h
@@ -25,6 +25,8 @@ int is_hugepage_mem_enough(size_t);
 unsigned long hugetlb_total_pages(void);
 struct page *alloc_huge_page(void);
 void free_huge_page(struct page *);
+int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct * vma,
+			unsigned long address, int write_access);
 
 extern unsigned long max_huge_pages;
 extern const unsigned long hugetlb_zero, hugetlb_infinity;
mm/hugetlb.c +94 -45

160 lines not on this page · raw message

mm/memory.c +1 -1
diff -upN reference/mm/memory.c current/mm/memory.c
@@ -2040,7 +2040,7 @@ int __handle_mm_fault(struct mm_struct *
 	inc_page_state(pgfault);
 
 	if (is_vm_hugetlb_page(vma))
-		return VM_FAULT_SIGBUS;	/* mapping truncation does this. */
+		return hugetlb_fault(mm, vma, address, write_access);
 
 	/*
 	 * We need the page table lock to synchronize with kswapd
Signature · 8 lines
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#5 lore raw

Re: [PATCH 2/3] hugetlb: Demand fault handler

Quoted 11 lines · Adam Litke

Version 5 (Tue, 11 Oct 2005)
Deal with hugetlbfs file truncation in find_get_huge_page()
Version 4 (Mon, 03 Oct 2005)
Make find_get_huge_page bale properly when add_to_page_cache fails
due to OOM conditions
Version 3 (Thu, 08 Sep 2005)
Organized logic in hugetlb_pte_fault() by breaking out
find_get_page/alloc_huge_page logic into separate function
Removed a few more paranoid checks ( Thanks )
Fixed tlb flushing in a race case ( Yanmin Zhang )

Quoted 5 lines

Version 2 (Wed, 17 Aug 2005)
Removed spurious WARN_ON()
Patches added earlier in the series (now in mainline):
Check for p?d_none() in arch/i386/mm/hugetlbpage.c:huge_pte_offset() Move i386 stale pte check into huge_pte_alloc()

I'm not sure this does fully deal with truncation, I'm afraid - it will deal with a truncation well before the fault, but not a concurrent truncate(). We'll need the truncate_count/retry logic from do_no_page, I think. Andi/Hugh, can you confirm that's correct?

Quoted 9 lines

Initial Post (Fri, 05 Aug 2005)

Below is a patch to implement demand faulting for huge pages. The main motivation for changing from prefaulting to demand faulting is so that huge page memory areas can be allocated according to NUMA policy.

Thanks to consolidated hugetlb code, switching the behavior requires changing only one fault handler. The bulk of the patch just moves the logic from hugelb_prefault() to hugetlb_pte_fault() and find_get_huge_page().

While we're at it - it's a minor nit, but I find the distinction between hugetlb_pte_fault() and hugetlb_fault() confusing. A better name for the former would be hugetlb_no_page(), in which case we should probably also move the border between it and hugetlb_find_get_page() to match the boundary between do_no_page() and mapping->nopage.

How about this, for example:

fs/hugetlbfs/inode.c +1 -5
Index: working-2.6/fs/hugetlbfs/inode.c
===================================================================
@@ -48,7 +48,6 @@
 static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct inode *inode = file->f_dentry->d_inode;
-	struct address_space *mapping = inode->i_mapping;
 	loff_t len, vma_len;
 	int ret;
 
@@ -79,10 +78,7 @@
 	if (!(vma->vm_flags & VM_WRITE) && len > inode->i_size)
 		goto out;
 
-	ret = hugetlb_prefault(mapping, vma);
-	if (ret)
-		goto out;
-
+	ret = 0;
 	if (inode->i_size < len)
 		inode->i_size = len;
 out:
include/linux/hugetlb.h +2 -0
Index: working-2.6/include/linux/hugetlb.h
===================================================================
@@ -25,6 +25,8 @@
 unsigned long hugetlb_total_pages(void);
 struct page *alloc_huge_page(void);
 void free_huge_page(struct page *);
+int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct * vma,
+			unsigned long address, int write_access);
 
 extern unsigned long max_huge_pages;
 extern const unsigned long hugetlb_zero, hugetlb_infinity;
mm/hugetlb.c +113 -43

179 lines not on this page · raw message

mm/memory.c +1 -1
Index: working-2.6/mm/memory.c
===================================================================
@@ -2040,7 +2040,7 @@
 	inc_page_state(pgfault);
 
 	if (is_vm_hugetlb_page(vma))
-		return VM_FAULT_SIGBUS;	/* mapping truncation does this. */
+		return hugetlb_fault(mm, vma, address, write_access);
 
 	/*
 	 * We need the page table lock to synchronize with kswapd
Signature · 10 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/people/dgibson

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#6 lore raw

Re: [PATCH 2/3] hugetlb: Demand fault handler

Quoted 5 lines · David Gibson

I'm not sure this does fully deal with truncation, I'm afraid - it will deal with a truncation well before the fault, but not a concurrent truncate(). We'll need the truncate_count/retry logic from do_no_page, I think. Andi/Hugh, can you confirm that's correct?

Very likely you're right, but I already explained to Adam privately that I won't get to look at all this before tomorrow - sorry.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#7 lore raw

Re: [PATCH 2/3] hugetlb: Demand fault handler

In reply to David Gibson

Thanks for the review and comments...

Quoted 18 lines · David Gibson
Quoted 11 lines · Adam Litke

Version 5 (Tue, 11 Oct 2005)
Deal with hugetlbfs file truncation in find_get_huge_page()
Version 4 (Mon, 03 Oct 2005)
Make find_get_huge_page bale properly when add_to_page_cache fails
due to OOM conditions
Version 3 (Thu, 08 Sep 2005)
Organized logic in hugetlb_pte_fault() by breaking out
find_get_page/alloc_huge_page logic into separate function
Removed a few more paranoid checks ( Thanks )
Fixed tlb flushing in a race case ( Yanmin Zhang )

Quoted 5 lines

Version 2 (Wed, 17 Aug 2005)
Removed spurious WARN_ON()
Patches added earlier in the series (now in mainline):
Check for p?d_none() in arch/i386/mm/hugetlbpage.c:huge_pte_offset() Move i386 stale pte check into huge_pte_alloc()

Quoted 4 lines

I'm not sure this does fully deal with truncation, I'm afraid - it will deal with a truncation well before the fault, but not a concurrent truncate(). We'll need the truncate_count/retry logic from do_no_page, I think. Andi/Hugh, can you confirm that's correct?

Ok. I can see why we need that.

Quoted 10 lines
Quoted 9 lines

Initial Post (Fri, 05 Aug 2005)

Below is a patch to implement demand faulting for huge pages. The main motivation for changing from prefaulting to demand faulting is so that huge page memory areas can be allocated according to NUMA policy.

Thanks to consolidated hugetlb code, switching the behavior requires changing only one fault handler. The bulk of the patch just moves the logic from hugelb_prefault() to hugetlb_pte_fault() and find_get_huge_page().

Quoted 8 lines

While we're at it - it's a minor nit, but I find the distinction between hugetlb_pte_fault() and hugetlb_fault() confusing. A better name for the former would be hugetlb_no_page(), in which case we should probably also move the border between it and hugetlb_find_get_page() to match the boundary between do_no_page() and mapping->nopage.

How about this, for example:

Yeah, I suppose that division makes more sense when comparing to the normal fault handler code.

Quoted 6 lines
mm/hugetlb.c +1 -1
@@ -338,57 +337,128 @@
 	spin_unlock(&mm->page_table_lock);
 }
 
-int hugetlb_prefault(struct address_space *mapping, struct vm_area_struct *vma)
+static struct page *hugetlbfs_nopage(struct vm_area_struct *vma,

<snip>

mm/hugetlb.c line 361

Quoted 10 lines
mm/hugetlb.c +10 -0
+	/* Check to make sure the mapping hasn't been truncated */
+	size = i_size_read(inode) >> HPAGE_SHIFT;
+	if (pgoff >= size)
+		return NULL;
+
+ retry:
+	page = find_get_page(mapping, pgoff);
+	if (page)
+		/* Another thread won the race */
+		return page;

Both of those returns could be changed to goto out so that the function has only one exit path. Isn't that what we want?

Signature · 8 lines
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#8 lore raw

Re: [PATCH 2/3] hugetlb: Demand fault handler

Quoted 59 lines · Adam Litke

Thanks for the review and comments...

Quoted 18 lines · David Gibson
Quoted 11 lines · Adam Litke

Version 5 (Tue, 11 Oct 2005)
Deal with hugetlbfs file truncation in find_get_huge_page()
Version 4 (Mon, 03 Oct 2005)
Make find_get_huge_page bale properly when add_to_page_cache fails
due to OOM conditions
Version 3 (Thu, 08 Sep 2005)
Organized logic in hugetlb_pte_fault() by breaking out
find_get_page/alloc_huge_page logic into separate function
Removed a few more paranoid checks ( Thanks )
Fixed tlb flushing in a race case ( Yanmin Zhang )

Quoted 5 lines

Version 2 (Wed, 17 Aug 2005)
Removed spurious WARN_ON()
Patches added earlier in the series (now in mainline):
Check for p?d_none() in arch/i386/mm/hugetlbpage.c:huge_pte_offset() Move i386 stale pte check into huge_pte_alloc()

Quoted 4 lines

I'm not sure this does fully deal with truncation, I'm afraid - it will deal with a truncation well before the fault, but not a concurrent truncate(). We'll need the truncate_count/retry logic from do_no_page, I think. Andi/Hugh, can you confirm that's correct?

Ok. I can see why we need that.

Quoted 10 lines
Quoted 9 lines

Initial Post (Fri, 05 Aug 2005)

Below is a patch to implement demand faulting for huge pages. The main motivation for changing from prefaulting to demand faulting is so that huge page memory areas can be allocated according to NUMA policy.

Thanks to consolidated hugetlb code, switching the behavior requires changing only one fault handler. The bulk of the patch just moves the logic from hugelb_prefault() to hugetlb_pte_fault() and find_get_huge_page().

Quoted 8 lines

While we're at it - it's a minor nit, but I find the distinction between hugetlb_pte_fault() and hugetlb_fault() confusing. A better name for the former would be hugetlb_no_page(), in which case we should probably also move the border between it and hugetlb_find_get_page() to match the boundary between do_no_page() and mapping->nopage.

How about this, for example:

Yeah, I suppose that division makes more sense when comparing to the normal fault handler code.

Quoted 6 lines
(quoted diff) +1 -1
@@ -338,57 +337,128 @@
 	spin_unlock(&mm->page_table_lock);
 }
 
-int hugetlb_prefault(struct address_space *mapping, struct vm_area_struct *vma)
+static struct page *hugetlbfs_nopage(struct vm_area_struct *vma,

<snip>

Quoted 13 lines

Review

Quoted 10 lines
(quoted diff) +10 -0
+	/* Check to make sure the mapping hasn't been truncated */
+	size = i_size_read(inode) >> HPAGE_SHIFT;
+	if (pgoff >= size)
+		return NULL;
+
+ retry:
+	page = find_get_page(mapping, pgoff);
+	if (page)
+		/* Another thread won the race */
+		return page;

Both of those returns could be changed to goto out so that the function has only one exit path. Isn't that what we want?

Eh, I've never been of the "gotos are better than multiple exit points" school of thought. However, I've not been consistent here, sometimes using return and sometimes goto out. Here's a new version

Subject: [PATCH 2/3] hugetlb: Demand fault handler
From: Adam Litke <[email protected]>
To: [email protected]
Cc: [email protected], [email protected],
David Gibson <[email protected]>, [email protected], [email protected]
Organization: IBM
Date: Tue, 11 Oct 2005 13:32:38 -0500

Version 5 (Tue, 11 Oct 2005)
Deal with hugetlbfs file truncation in find_get_huge_page()
Version 4 (Mon, 03 Oct 2005)
Make find_get_huge_page bale properly when add_to_page_cache fails
due to OOM conditions
Version 3 (Thu, 08 Sep 2005)
Organized logic in hugetlb_pte_fault() by breaking out
find_get_page/alloc_huge_page logic into separate function
Removed a few more paranoid checks ( Thanks )
Fixed tlb flushing in a race case ( Yanmin Zhang )

Version 2 (Wed, 17 Aug 2005)
Removed spurious WARN_ON()
Patches added earlier in the series (now in mainline):
Check for p?d_none() in arch/i386/mm/hugetlbpage.c:huge_pte_offset() Move i386 stale pte check into huge_pte_alloc()

Initial Post (Fri, 05 Aug 2005)

Below is a patch to implement demand faulting for huge pages. The main motivation for changing from prefaulting to demand faulting is so that huge page memory areas can be allocated according to NUMA policy.

Thanks to consolidated hugetlb code, switching the behavior requires changing only one fault handler. The bulk of the patch just moves the logic from hugelb_prefault() to hugetlb_pte_fault() and find_get_huge_page().

fs/hugetlbfs/inode.c +1 -5
Index: working-2.6/fs/hugetlbfs/inode.c
===================================================================
@@ -48,7 +48,6 @@
 static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct inode *inode = file->f_dentry->d_inode;
-	struct address_space *mapping = inode->i_mapping;
 	loff_t len, vma_len;
 	int ret;
 
@@ -79,10 +78,7 @@
 	if (!(vma->vm_flags & VM_WRITE) && len > inode->i_size)
 		goto out;
 
-	ret = hugetlb_prefault(mapping, vma);
-	if (ret)
-		goto out;
-
+	ret = 0;
 	if (inode->i_size < len)
 		inode->i_size = len;
 out:
include/linux/hugetlb.h +2 -0
Index: working-2.6/include/linux/hugetlb.h
===================================================================
@@ -25,6 +25,8 @@
 unsigned long hugetlb_total_pages(void);
 struct page *alloc_huge_page(void);
 void free_huge_page(struct page *);
+int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct * vma,
+			unsigned long address, int write_access);
 
 extern unsigned long max_huge_pages;
 extern const unsigned long hugetlb_zero, hugetlb_infinity;
mm/hugetlb.c +114 -45

180 lines not on this page · raw message

mm/memory.c +1 -1
Index: working-2.6/mm/memory.c
===================================================================
@@ -2040,7 +2040,7 @@
 	inc_page_state(pgfault);
 
 	if (is_vm_hugetlb_page(vma))
-		return VM_FAULT_SIGBUS;	/* mapping truncation does this. */
+		return hugetlb_fault(mm, vma, address, write_access);
 
 	/*
 	 * We need the page table lock to synchronize with kswapd
Signature · 10 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/people/dgibson

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#9 lore raw

PATCH 3/3 hugetlb: Simple overcommit check

In reply to Adam Litke

Initial Post (Thu, 18 Aug 2005)

Basic overcommit checking for hugetlb_file_map() based on an implementation used with demand faulting in SLES9.

Since we're not prefaulting the pages at mmap time, some extra accounting is needed. This patch implements a basic sanity check to ensure that the number of huge pages required to satisfy the mmap are currently available. Of course this method doesn't guarantee that the pages will be available at fault time, but I think it is a good start on doing proper accounting and solves 90% of the overcommit problems I see in practice.

Huge page shared memory segments are simpler and still maintain their commit on shmget semantics.

---
 inode.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 55 insertions(+), 10 deletions(-)
fs/hugetlbfs/inode.c +55 -10
diff -upN reference/fs/hugetlbfs/inode.c current/fs/hugetlbfs/inode.c
@@ -45,9 +45,59 @@ static struct backing_dev_info hugetlbfs
 
 int sysctl_hugetlb_shm_group;
 
+static void huge_pagevec_release(struct pagevec *pvec)
+{
+	int i;
+
+	for (i = 0; i < pagevec_count(pvec); ++i)
+		put_page(pvec->pages[i]);
+
+	pagevec_reinit(pvec);
+}
+
+unsigned long
+huge_pages_needed(struct address_space *mapping, struct vm_area_struct *vma)
+{
+	int i;
+	struct pagevec pvec;
+	unsigned long start = vma->vm_start;
+	unsigned long end = vma->vm_end;
+	unsigned long hugepages = (end - start) >> HPAGE_SHIFT;
+	pgoff_t next = vma->vm_pgoff;
+	pgoff_t endpg = next + ((end - start) >> PAGE_SHIFT);
+	struct inode *inode = vma->vm_file->f_dentry->d_inode;
+
+	/*
+	 * Shared memory segments are accounted for at shget time,
+	 * not at shmat (when the mapping is actually created) so 
+	 * check here if the memory has already been accounted for.
+	 */
+	if (inode->i_blocks != 0)
+		return 0;
+
+	pagevec_init(&pvec, 0);
+	while (next < endpg) {
+		if (!pagevec_lookup(&pvec, mapping, next, PAGEVEC_SIZE))
+			break;
+		for (i = 0; i < pagevec_count(&pvec); i++) {
+			struct page *page = pvec.pages[i];
+			if (page->index > next)
+				next = page->index;
+			if (page->index >= endpg)
+				break;
+			next++;
+			hugepages--;
+		}
+		huge_pagevec_release(&pvec);
+	}
+	return hugepages << HPAGE_SHIFT;
+}
+
 static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct inode *inode = file->f_dentry->d_inode;
+	struct address_space *mapping = inode->i_mapping;
+	unsigned long bytes;
 	loff_t len, vma_len;
 	int ret;
 
@@ -66,6 +116,10 @@ static int hugetlbfs_file_mmap(struct fi
 	if (vma->vm_end - vma->vm_start < HPAGE_SIZE)
 		return -EINVAL;
 
+	bytes = huge_pages_needed(mapping, vma);
+	if (!is_hugepage_mem_enough(bytes))
+		return -ENOMEM;
+
 	vma_len = (loff_t)(vma->vm_end - vma->vm_start);
 
 	down(&inode->i_sem);
@@ -167,16 +221,6 @@ static int hugetlbfs_commit_write(struct
 	return -EINVAL;
 }
 
-static void huge_pagevec_release(struct pagevec *pvec)
-{
-	int i;
-
-	for (i = 0; i < pagevec_count(pvec); ++i)
-		put_page(pvec->pages[i]);
-
-	pagevec_reinit(pvec);
-}
-
 static void truncate_huge_page(struct page *page)
 {
 	clear_page_dirty(page);
@@ -792,6 +836,7 @@ struct file *hugetlb_zero_setup(size_t s
 	d_instantiate(dentry, inode);
 	inode->i_size = size;
 	inode->i_nlink = 0;
+	inode->i_blocks = 1;
 	file->f_vfsmnt = mntget(hugetlbfs_vfsmount);
 	file->f_dentry = dentry;
 	file->f_mapping = inode->i_mapping;
Signature · 8 lines
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>