hugetlb: copy on write

PATCH In review Adam Litke 4 participants 14 messages Nov 9, 2005 – Nov 10, 2005

Subsystems (3)

Memory management Memory mapping HugeTLB

lore

Share & tools

lore

Adam Litke

author

2005-11-09 23:29 UTC #0 lore raw

cover hugetlb: copy on write

This is a resend of the patches I sent on Nov 7th. I've broken them out as requested. Comments (especially on the copy-on-write portion) appreciated. Does anyone have a fundamental objection to moving forward with these?

remove-dup-isize-check - Remove duplicated i_size truncation race check rename-find_lock_huge_page - Switch to a more appropriate name hugetlb_no_page - Mild reorg to support multiple fault types htlb-cow - Copy on write support

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>

Adam Litke

author

2005-11-09 23:37 UTC #1 lore raw

PATCH 1/4 Hugetlb: Remove duplicate i_size check

Quoted 4 lines · David Gibson

- The check against i_size was duplicated: once in find_lock_huge_page() and again in hugetlb_fault() after taking the page_table_lock. We only really need the locked one, so remove the other.

Original post by David Gibson <[email protected]>

Version 2: Wed 9 Nov 2005
Split this cleanup out into a standalone patch

Signed-off-by David Gibson <[email protected]>

Signed-off-by Adam Litke <[email protected]>

---
 hugetlb.c |    7 -------
 1 files changed, 7 deletions(-)

mm/hugetlb.c +0 -7

diff -upN reference/mm/hugetlb.c current/mm/hugetlb.c
@@ -344,19 +344,12 @@ static struct page *find_lock_huge_page(
 {
 	struct page *page;
 	int err;
-	struct inode *inode = mapping->host;
-	unsigned long size;
 
 retry:
 	page = find_lock_page(mapping, idx);
 	if (page)
 		goto out;
 
-	/* Check to make sure the mapping hasn't been truncated */
-	size = i_size_read(inode) >> HPAGE_SHIFT;
-	if (idx >= size)
-		goto out;
-
 	if (hugetlb_get_quota(mapping))
 		goto out;
 	page = alloc_huge_page();

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>

William Lee Irwin III

Acked-by on v1 1/4

2005-11-10 00:10 UTC #2 lore raw

Re: [PATCH 1/4] Hugetlb: Remove duplicate i_size check

Quoted 4 lines · David Gibson

Quoted 4 lines
- The check against i_size was duplicated: once in find_lock_huge_page() and again in hugetlb_fault() after taking the page_table_lock. We only really need the locked one, so remove the other.

Quoted 5 lines · Adam Litke

Original post by David Gibson <[email protected]> Version 2: Wed 9 Nov 2005
Split this cleanup out into a standalone patch
Signed-off-by David Gibson <[email protected]>
Signed-off-by Adam Litke <[email protected]>

Innocuous enough.

Acked-by William Irwin <[email protected]>

--
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>

Adam Litke

author

2005-11-09 23:38 UTC #3 lore raw

PATCH 2/4 Hugetlb: Rename find_lock_page to find_or_alloc_huge_page

In reply to Adam Litke

On Wed, 2005-10-26 at 12:00 +1000, David Gibson wrote:
- find_lock_huge_page() isn't a great name, since it does extra things not analagous to find_lock_page(). Rename it find_or_alloc_huge_page() which is closer to the mark.

Original post by David Gibson <[email protected]>

Version 2: Wed 9 Nov 2005
Split into a separate patch

Signed-off-by David Gibson <[email protected]>

Signed-off-by Adam Litke <[email protected]>

---
 hugetlb.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

mm/hugetlb.c +3 -3

diff -upN reference/mm/hugetlb.c current/mm/hugetlb.c
@@ -339,8 +339,8 @@ void unmap_hugepage_range(struct vm_area
 	flush_tlb_range(vma, start, end);
 }
 
-static struct page *find_lock_huge_page(struct address_space *mapping,
-			unsigned long idx)
+static struct page *find_or_alloc_huge_page(struct address_space *mapping,
+						unsigned long idx)
 {
 	struct page *page;
 	int err;

@@ -392,7 +392,7 @@ int hugetlb_fault(struct mm_struct *mm, 
 	 * Use page lock to guard against racing truncation
 	 * before we get page_table_lock.
 	 */
-	page = find_lock_huge_page(mapping, idx);
+	page = find_or_alloc_huge_page(mapping, idx);
 	if (!page)
 		goto out;

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>

William Lee Irwin III

Acked-by on v1 2/4

2005-11-10 00:11 UTC #4 lore raw

Re: [PATCH 2/4] Hugetlb: Rename find_lock_page to find_or_alloc_huge_page

Quoted 11 lines · Adam Litke

Hugetlb: Rename find_lock_page to find_or_alloc_huge_page
On Wed, 2005-10-26 at 12:00 +1000, David Gibson wrote:
- find_lock_huge_page() isn't a great name, since it does extra things not analagous to find_lock_page(). Rename it find_or_alloc_huge_page() which is closer to the mark.
Original post by David Gibson <[email protected]> Version 2: Wed 9 Nov 2005
Split into a separate patch
Signed-off-by David Gibson <[email protected]>
Signed-off-by Adam Litke <[email protected]>

Also innocuous.

Acked-by William Irwin <[email protected]>

-- wli

Adam Litke

author

2005-11-09 23:39 UTC #5 lore raw

PATCH 3/4 Hugetlb: Reorganize hugetlb_fault to prepare for COW

In reply to Adam Litke

This patch splits the "no_page()" type activity into its own function, hugetlb_no_page(). hugetlb_fault() becomes the entry point for hugetlb faults and delegates to the appropriate handler depending on the type of fault. Right now we still have only hugetlb_no_page() but a later patch introduces a COW fault.

Original post by David Gibson <[email protected]>

Version 2: Wed 9 Nov 2005
Broken out into a separate patch

Signed-off-by David Gibson <[email protected]>

Signed-off-by Adam Litke <[email protected]>

---
 hugetlb.c |   34 +++++++++++++++++++++++++---------
 1 files changed, 25 insertions(+), 9 deletions(-)

mm/hugetlb.c +25 -9

diff -upN reference/mm/hugetlb.c current/mm/hugetlb.c
@@ -370,20 +370,15 @@ out:
 	return page;
 }
 
-int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
-			unsigned long address, int write_access)
+int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
+			unsigned long address, pte_t *ptep)
 {
 	int ret = VM_FAULT_SIGBUS;
 	unsigned long idx;
 	unsigned long size;
-	pte_t *pte;
 	struct page *page;
 	struct address_space *mapping;
 
-	pte = huge_pte_alloc(mm, address);
-	if (!pte)
-		goto out;
-
 	mapping = vma->vm_file->f_mapping;
 	idx = ((address - vma->vm_start) >> HPAGE_SHIFT)
 		+ (vma->vm_pgoff >> (HPAGE_SHIFT - PAGE_SHIFT));

@@ -402,11 +397,11 @@ int hugetlb_fault(struct mm_struct *mm, 
 		goto backout;
 
 	ret = VM_FAULT_MINOR;
-	if (!pte_none(*pte))
+	if (!pte_none(*ptep))
 		goto backout;
 
 	add_mm_counter(mm, file_rss, HPAGE_SIZE / PAGE_SIZE);
-	set_huge_pte_at(mm, address, pte, make_huge_pte(vma, page));
+	set_huge_pte_at(mm, address, ptep, make_huge_pte(vma, page));
 	spin_unlock(&mm->page_table_lock);
 	unlock_page(page);
 out:

@@ -420,6 +415,27 @@ backout:
 	goto out;
 }
 
+int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
+			unsigned long address, int write_access)
+{
+	pte_t *ptep;
+	pte_t entry;
+
+	ptep = huge_pte_alloc(mm, address);
+	if (!ptep)
+		return VM_FAULT_OOM;
+
+	entry = *ptep;
+	if (pte_none(entry))
+		return hugetlb_no_page(mm, vma, address, ptep);
+
+	/*
+	 * We could get here if another thread instantiated the pte
+	 * before the test above.
+	 */
+	return VM_FAULT_MINOR;
+}
+
 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)

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>

William Lee Irwin III

Acked-by on v1 3/4

2005-11-10 00:13 UTC #6 lore raw

Re: [PATCH 3/4] Hugetlb: Reorganize hugetlb_fault to prepare for COW

Quoted 11 lines · Adam Litke

Hugetlb: Reorganize hugetlb_fault to prepare for COW
This patch splits the "no_page()" type activity into its own function, hugetlb_no_page(). hugetlb_fault() becomes the entry point for hugetlb faults and delegates to the appropriate handler depending on the type of fault. Right now we still have only hugetlb_no_page() but a later patch introduces a COW fault.
Original post by David Gibson <[email protected]> Version 2: Wed 9 Nov 2005
Broken out into a separate patch
Signed-off-by David Gibson <[email protected]>
Signed-off-by Adam Litke <[email protected]>

Straightforward enough.

Acked-by William Irwin <[email protected]>

-- wli

Adam Litke

author

2005-11-09 23:40 UTC #7 lore raw

PATCH 4/4 Hugetlb: Copy on Write support

In reply to Adam Litke

Implement copy-on-write support for hugetlb mappings so MAP_PRIVATE can be supported. This helps us to safely use hugetlb pages in many more applications. The patch makes the following changes. If needed, I also have it broken out according to the following paragraphs.

1. Add a pair of functions to set/clear write access on huge ptes. The writable check in make_huge_pte is moved out to the caller for use by COW later.

2. Hugetlb copy-on-write requires special case handling in the following situations:
- copy_hugetlb_page_range() - Copied pages must be write protected so a COW fault will be triggered (if necessary) if those pages are written to.
- find_or_alloc_huge_page() - Only MAP_SHARED pages are added to the page cache. MAP_PRIVATE pages still need to be locked however.

3. Provide hugetlb_cow() and calls from hugetlb_fault() and hugetlb_no_page() which handles the COW fault by making the actual copy.

4. Remove the check in hugetlbfs_file_map() so that MAP_PRIVATE mmaps will be allowed. Make MAP_HUGETLB exempt from the depricated VM_RESERVED mapping check.

Signed-off-by David Gibson <[email protected]>

Signed-off-by Adam Litke <[email protected]>

---
 fs/hugetlbfs/inode.c    |    3 -
 include/linux/hugetlb.h |   12 +++++
 mm/hugetlb.c            |  115 ++++++++++++++++++++++++++++++++++++++++--------
 mm/mmap.c               |    4 -
 4 files changed, 110 insertions(+), 24 deletions(-)

fs/hugetlbfs/inode.c +0 -3

diff -upN reference/fs/hugetlbfs/inode.c current/fs/hugetlbfs/inode.c
@@ -100,9 +100,6 @@ static int hugetlbfs_file_mmap(struct fi
 	loff_t len, vma_len;
 	int ret;
 
-	if ((vma->vm_flags & (VM_MAYSHARE | VM_WRITE)) == VM_WRITE)
-		return -EINVAL;
-
 	if (vma->vm_pgoff & (HPAGE_SIZE / PAGE_SIZE - 1))
 		return -EINVAL;

include/linux/hugetlb.h +12 -0

diff -upN reference/include/linux/hugetlb.h current/include/linux/hugetlb.h
@@ -65,6 +65,18 @@ pte_t huge_ptep_get_and_clear(struct mm_
 			      pte_t *ptep);
 #endif
 
+#define huge_ptep_set_wrprotect(mm, addr, ptep) \
+	ptep_set_wrprotect(mm, addr, ptep)
+static inline void set_huge_ptep_writable(struct vm_area_struct *vma,
+		unsigned long address, pte_t *ptep)
+{
+	pte_t entry;
+
+	entry = pte_mkwrite(pte_mkdirty(*ptep));
+	ptep_set_access_flags(vma, address, ptep, entry, 1);
+	update_mmu_cache(vma, address, entry);
+}
+
 #ifndef ARCH_HAS_HUGETLB_PREFAULT_HOOK
 #define hugetlb_prefault_arch_hook(mm)		do { } while (0)
 #else

mm/hugetlb.c +96 ~~-19~~

188 lines not on this page · raw message

mm/mmap.c +2 -2

diff -upN reference/mm/mmap.c current/mm/mmap.c
@@ -1077,8 +1077,8 @@ munmap_back:
 		error = file->f_op->mmap(file, vma);
 		if (error)
 			goto unmap_and_free_vma;
-		if ((vma->vm_flags & (VM_SHARED | VM_WRITE | VM_RESERVED))
-						== (VM_WRITE | VM_RESERVED)) {
+		if ((vma->vm_flags & (VM_SHARED | VM_WRITE | VM_RESERVED 
+				| VM_HUGETLB)) == (VM_WRITE | VM_RESERVED)) {
 			printk(KERN_WARNING "program %s is using MAP_PRIVATE, "
 				"PROT_WRITE mmap of VM_RESERVED memory, which "
 				"is deprecated. Please report this to "

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>

William Lee Irwin III

2005-11-10 00:15 UTC #8 lore raw

Re: [PATCH 4/4] Hugetlb: Copy on Write support

Quoted 11 lines · Adam Litke

Hugetlb: Copy on Write support
Implement copy-on-write support for hugetlb mappings so MAP_PRIVATE can be supported. This helps us to safely use hugetlb pages in many more applications. The patch makes the following changes. If needed, I also have it broken out according to the following paragraphs.
1. Add a pair of functions to set/clear write access on huge ptes. The writable check in make_huge_pte is moved out to the caller for use by COW later.
2. Hugetlb copy-on-write requires special case handling in the following situations:
- copy_hugetlb_page_range() - Copied pages must be write protected so a COW

Quoted 8 lines

fault will be triggered (if necessary) if those pages are written to.
- find_or_alloc_huge_page() - Only MAP_SHARED pages are added to the page cache. MAP_PRIVATE pages still need to be locked however.
3. Provide hugetlb_cow() and calls from hugetlb_fault() and hugetlb_no_page() which handles the COW fault by making the actual copy.
4. Remove the check in hugetlbfs_file_map() so that MAP_PRIVATE mmaps will be allowed. Make MAP_HUGETLB exempt from the depricated VM_RESERVED mapping check.

Did you do the audit of pte protection bits I asked about? If not, I'll dredge them up and check to make sure.

-- wli

David Gibson

2005-11-10 00:49 UTC #9 lore raw

Re: [PATCH 4/4] Hugetlb: Copy on Write support

Quoted 21 lines · William Lee Irwin

Quoted 11 lines · Adam Litke
Hugetlb: Copy on Write support
Implement copy-on-write support for hugetlb mappings so MAP_PRIVATE can be supported. This helps us to safely use hugetlb pages in many more applications. The patch makes the following changes. If needed, I also have it broken out according to the following paragraphs.
1. Add a pair of functions to set/clear write access on huge ptes. The writable check in make_huge_pte is moved out to the caller for use by COW later.
2. Hugetlb copy-on-write requires special case handling in the following situations:
- copy_hugetlb_page_range() - Copied pages must be write protected so a COW
Quoted 8 lines
fault will be triggered (if necessary) if those pages are written to.
- find_or_alloc_huge_page() - Only MAP_SHARED pages are added to the page cache. MAP_PRIVATE pages still need to be locked however.
3. Provide hugetlb_cow() and calls from hugetlb_fault() and hugetlb_no_page() which handles the COW fault by making the actual copy.
4. Remove the check in hugetlbfs_file_map() so that MAP_PRIVATE mmaps will be allowed. Make MAP_HUGETLB exempt from the depricated VM_RESERVED mapping check.

Quoted 2 lines

Did you do the audit of pte protection bits I asked about? If not, I'll dredge them up and check to make sure.

I still don't know what you're talking about here - you never responded to my mail asking for clarification. The hugepage code already relies on pte_mkwrite() and pte_wrprotect() working correctly, I don't see that COW makes any difference.

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/~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>

William Lee Irwin III

Acked-by

2005-11-10 00:56 UTC #10 lore raw

Re: [PATCH 4/4] Hugetlb: Copy on Write support

Quoted 2 lines · William Lee Irwin

Quoted 2 lines
Did you do the audit of pte protection bits I asked about? If not, I'll dredge them up and check to make sure.

Quoted 4 lines · David Gibson

I still don't know what you're talking about here - you never responded to my mail asking for clarification. The hugepage code already relies on pte_mkwrite() and pte_wrprotect() working correctly, I don't see that COW makes any difference.

You appear to have a good idea of what's going on given that you've reminded me of that reliance. It looks like I dropped that email packet for some reason, sorry about that.

Acked-by William Irwin <[email protected]>

-- wli

Rohit Seth

2005-11-10 01:47 UTC #11 lore raw

Re: [PATCH 4/4] Hugetlb: Copy on Write support

In reply to Adam Litke

include/linux/hugetlb.h pte_t huge_ptep_get_and_clear(struct mm_ · line 78

Quoted 12 lines · Adam Litke

include/linux/hugetlb.h +11 -0

+#define huge_ptep_set_wrprotect(mm, addr, ptep) \
+	ptep_set_wrprotect(mm, addr, ptep)
+static inline void set_huge_ptep_writable(struct vm_area_struct *vma,
+		unsigned long address, pte_t *ptep)
+{
+	pte_t entry;
+
+	entry = pte_mkwrite(pte_mkdirty(*ptep));
+	ptep_set_access_flags(vma, address, ptep, entry, 1);
+	update_mmu_cache(vma, address, entry);
+}

lazy_mmu_prot_update will need to called here to make caches coherent for some archs.

-rohit

David Gibson

2005-11-10 03:54 UTC #12 lore raw

Re: [PATCH 4/4] Hugetlb: Copy on Write support

Quoted 17 lines · Rohit Seth

Review

Quoted 12 lines · Adam Litke

(quoted diff) +11 -0

+#define huge_ptep_set_wrprotect(mm, addr, ptep) \
+	ptep_set_wrprotect(mm, addr, ptep)
+static inline void set_huge_ptep_writable(struct vm_area_struct *vma,
+		unsigned long address, pte_t *ptep)
+{
+	pte_t entry;
+
+	entry = pte_mkwrite(pte_mkdirty(*ptep));
+	ptep_set_access_flags(vma, address, ptep, entry, 1);
+	update_mmu_cache(vma, address, entry);
+}

lazy_mmu_prot_update will need to called here to make caches coherent for some archs.

Ah, yes indeed. Revised version below. While I was at it, I moved set_huge_ptep_writable() into mm/hugetlb.c, since there's no actual need for it to be in the .h, and abolished huge_ptep_set_wrprotect() since there's no need for the macro at all.

Hugetlb: Copy on Write support

1. Add a pair of functions to set/clear write access on huge ptes. The writable check in make_huge_pte is moved out to the caller for use by COW later.

3. Provide hugetlb_cow() and calls from hugetlb_fault() and hugetlb_no_page() which handles the COW fault by making the actual copy.

4. Remove the check in hugetlbfs_file_map() so that MAP_PRIVATE mmaps will be allowed. Make MAP_HUGETLB exempt from the depricated VM_RESERVED mapping check.

Signed-off-by David Gibson <[email protected]>

Signed-off-by Adam Litke <[email protected]>

fs/hugetlbfs/inode.c +0 -3

Index: working-2.6/fs/hugetlbfs/inode.c
===================================================================
@@ -100,9 +100,6 @@
 	loff_t len, vma_len;
 	int ret;
 
-	if ((vma->vm_flags & (VM_MAYSHARE | VM_WRITE)) == VM_WRITE)
-		return -EINVAL;
-
 	if (vma->vm_pgoff & (HPAGE_SIZE / PAGE_SIZE - 1))
 		return -EINVAL;

mm/hugetlb.c +108 ~~-19~~

206 lines not on this page · raw message

mm/mmap.c +3 -2

Index: working-2.6/mm/mmap.c
===================================================================
@@ -1076,8 +1076,9 @@
 		error = file->f_op->mmap(file, vma);
 		if (error)
 			goto unmap_and_free_vma;
-		if ((vma->vm_flags & (VM_SHARED | VM_WRITE | VM_RESERVED))
-						== (VM_WRITE | VM_RESERVED)) {
+		if ((vma->vm_flags
+		     & (VM_SHARED | VM_WRITE | VM_RESERVED | VM_HUGETLB))
+		    == (VM_WRITE | VM_RESERVED)) {
 			printk(KERN_WARNING "program %s is using MAP_PRIVATE, "
 				"PROT_WRITE mmap of VM_RESERVED memory, which "
 				"is deprecated. Please report this to "

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/~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>

William Lee Irwin III

Acked-by

2005-11-10 04:20 UTC #13 lore raw

Re: [PATCH 4/4] Hugetlb: Copy on Write support

Quoted 2 lines · Rohit Seth

Quoted 2 lines
lazy_mmu_prot_update will need to called here to make caches coherent for some archs.

Quoted 5 lines · David Gibson

Ah, yes indeed. Revised version below. While I was at it, I moved set_huge_ptep_writable() into mm/hugetlb.c, since there's no actual need for it to be in the .h, and abolished huge_ptep_set_wrprotect() since there's no need for the macro at all.
Hugetlb: Copy on Write support

Re-acking. Good catch, thanks Rohit.

Acked-by William Irwin <[email protected]>

-- wli