Re: [patch] mm: close page_mkwrite races

Re: [patch] mm: close page_mkwrite races

[Nick Piggin]

[Fixed linux-mm address. Please reply here]

Hi,

I'd like opinions on this patch (applies on top of the previous
page_mkwrite fixes in -mm). I was not going to ask to merge it
immediately however it appears that fsblock is not the only one who
needs it...
--

I want to have the page be protected by page lock between page_mkwrite
notification to the filesystem, and the actual setting of the page
dirty. Do this by allowing the filesystem to return a locked page from
page_mkwrite, and have the page fault code keep it held until after it
calls set_page_dirty.

I need this in fsblock because I am working to ensure filesystem metadata
can be correctly allocated and refcounted. This means that page cleaning
should not require memory allocation.

Without this patch, then for example we could get a concurrent writeout
after the page_mkwrite (which allocates page metadata required to clean
it), but before the set_page_dirty. The writeout will clean the page and
notice that the metadata is now unused and may be deallocated (because
it appears clean as set_page_dirty hasn't been called yet). So at this
point the page may be dirtied via the pte without enough metadata to be
able to write it back.

Sage needs this race closed for ceph, and Trond maybe for NFS.

Cc: Sage Weil <sage@newdream.net>
Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
Signed-off-by: Nick Piggin <npiggin@suse.de>

---
 Documentation/filesystems/Locking |   24 +++++++---
 fs/buffer.c                       |   10 ++--
 mm/memory.c                       |   83 ++++++++++++++++++++++++++------------
 3 files changed, 79 insertions(+), 38 deletions(-)

Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c
+++ linux-2.6/fs/buffer.c
@@ -2480,7 +2480,8 @@ block_page_mkwrite(struct vm_area_struct
 	if ((page->mapping != inode->i_mapping) ||
 	    (page_offset(page) > size)) {
 		/* page got truncated out from underneath us */
-		goto out_unlock;
+		unlock_page(page);
+		goto out;
 	}
 
 	/* page is wholly or partially inside EOF */
@@ -2494,14 +2495,15 @@ block_page_mkwrite(struct vm_area_struct
 		ret = block_commit_write(page, 0, end);
 
 	if (unlikely(ret)) {
+		unlock_page(page);
 		if (ret == -ENOMEM)
 			ret = VM_FAULT_OOM;
 		else /* -ENOSPC, -EIO, etc */
 			ret = VM_FAULT_SIGBUS;
-	}
+	} else
+		ret = VM_FAULT_LOCKED;
 
-out_unlock:
-	unlock_page(page);
+out:
 	return ret;
 }
 
Index: linux-2.6/mm/memory.c
===================================================================
--- linux-2.6.orig/mm/memory.c
+++ linux-2.6/mm/memory.c
@@ -1964,6 +1964,15 @@ static int do_wp_page(struct mm_struct *
 				ret = tmp;
 				goto unwritable_page;
 			}
+			if (unlikely(!(tmp & VM_FAULT_LOCKED))) {
+				lock_page(old_page);
+				if (!old_page->mapping) {
+					ret = 0; /* retry the fault */
+					unlock_page(old_page);
+					goto unwritable_page;
+				}
+			} else
+				VM_BUG_ON(!PageLocked(old_page));
 
 			/*
 			 * Since we dropped the lock we need to revalidate
@@ -1973,9 +1982,11 @@ static int do_wp_page(struct mm_struct *
 			 */
 			page_table = pte_offset_map_lock(mm, pmd, address,
 							 &ptl);
-			page_cache_release(old_page);
-			if (!pte_same(*page_table, orig_pte))
+			if (!pte_same(*page_table, orig_pte)) {
+				page_cache_release(old_page);
+				unlock_page(old_page);
 				goto unlock;
+			}
 
 			page_mkwrite = 1;
 		}
@@ -2098,16 +2109,30 @@ unlock:
 		 *
 		 * do_no_page is protected similarly.
 		 */
-		wait_on_page_locked(dirty_page);
-		set_page_dirty_balance(dirty_page, page_mkwrite);
+		if (!page_mkwrite) {
+			wait_on_page_locked(dirty_page);
+			set_page_dirty_balance(dirty_page, page_mkwrite);
+		}
 		put_page(dirty_page);
+		if (page_mkwrite) {
+			struct address_space *mapping = old_page->mapping;
+
+			unlock_page(old_page);
+			page_cache_release(old_page);
+			balance_dirty_pages_ratelimited(mapping);
+		}
 	}
 	return ret;
 oom_free_new:
 	page_cache_release(new_page);
 oom:
-	if (old_page)
+	if (old_page) {
+		if (page_mkwrite) {
+			unlock_page(old_page);
+			page_cache_release(old_page);
+		}
 		page_cache_release(old_page);
+	}
 	return VM_FAULT_OOM;
 
 unwritable_page:
@@ -2659,27 +2684,22 @@ static int __do_fault(struct mm_struct *
 				int tmp;
 
 				unlock_page(page);
-				vmf.flags |= FAULT_FLAG_MKWRITE;
+				vmf.flags = FAULT_FLAG_WRITE|FAULT_FLAG_MKWRITE;
 				tmp = vma->vm_ops->page_mkwrite(vma, &vmf);
 				if (unlikely(tmp &
 					  (VM_FAULT_ERROR | VM_FAULT_NOPAGE))) {
 					ret = tmp;
-					anon = 1; /* no anon but release vmf.page */
-					goto out_unlocked;
-				}
-				lock_page(page);
-				/*
-				 * XXX: this is not quite right (racy vs
-				 * invalidate) to unlock and relock the page
-				 * like this, however a better fix requires
-				 * reworking page_mkwrite locking API, which
-				 * is better done later.
-				 */
-				if (!page->mapping) {
-					ret = 0;
-					anon = 1; /* no anon but release vmf.page */
-					goto out;
+					goto unwritable_page;
 				}
+				if (unlikely(!(tmp & VM_FAULT_LOCKED))) {
+					lock_page(page);
+					if (!page->mapping) {
+						ret = 0; /* retry the fault */
+						unlock_page(page);
+						goto unwritable_page;
+					}
+				} else
+					VM_BUG_ON(!PageLocked(page));
 				page_mkwrite = 1;
 			}
 		}
@@ -2731,19 +2751,30 @@ static int __do_fault(struct mm_struct *
 	pte_unmap_unlock(page_table, ptl);
 
 out:
-	unlock_page(vmf.page);
-out_unlocked:
-	if (anon)
-		page_cache_release(vmf.page);
-	else if (dirty_page) {
+	if (dirty_page) {
+		struct address_space *mapping = page->mapping;
+
 		if (vma->vm_file)
 			file_update_time(vma->vm_file);
 
+		if (set_page_dirty(dirty_page))
+			page_mkwrite = 1;
 		set_page_dirty_balance(dirty_page, page_mkwrite);
+		unlock_page(dirty_page);
 		put_page(dirty_page);
+		if (page_mkwrite)
+			balance_dirty_pages_ratelimited(mapping);
+	} else {
+		unlock_page(vmf.page);
+		if (anon)
+			page_cache_release(vmf.page);
 	}
 
 	return ret;
+
+unwritable_page:
+	page_cache_release(page);
+	return ret;
 }
 
 static int do_linear_fault(struct mm_struct *mm, struct vm_area_struct *vma,
Index: linux-2.6/Documentation/filesystems/Locking
===================================================================
--- linux-2.6.orig/Documentation/filesystems/Locking
+++ linux-2.6/Documentation/filesystems/Locking
@@ -509,16 +509,24 @@ locking rules:
 		BKL	mmap_sem	PageLocked(page)
 open:		no	yes
 close:		no	yes
-fault:		no	yes
-page_mkwrite:	no	yes		no
+fault:		no	yes		can return with page locked
+page_mkwrite:	no	yes		can return with page locked
 access:		no	yes
 
-	->page_mkwrite() is called when a previously read-only page is
-about to become writeable. The file system is responsible for
-protecting against truncate races. Once appropriate action has been
-taking to lock out truncate, the page range should be verified to be
-within i_size. The page mapping should also be checked that it is not
-NULL.
+	->fault() is called when a previously not present pte is about
+to be faulted in. The filesystem must find and return the page associated
+with the passed in "pgoff" in the vm_fault structure. If it is possible that
+the page may be truncated and/or invalidated, then the filesystem must lock
+the page, then ensure it is not already truncated (the page lock will block
+subsequent truncate), and then return with VM_FAULT_LOCKED, and the page
+locked. The VM will unlock the page.
+
+	->page_mkwrite() is called when a previously read-only pte is
+about to become writeable. The filesystem again must ensure that there are
+no truncate/invalidate races, and then return with the page locked. If
+the page has been truncated, the filesystem should not look up a new page
+like the ->fault() handler, but simply return with VM_FAULT_NOPAGE, which
+will cause the VM to retry the fault.
 
 	->access() is called when get_user_pages() fails in
 acces_process_vm(), typically used to debug a process through

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

Re: [patch] mm: close page_mkwrite races

[Sage Weil]

On Mon, 30 Mar 2009, Nick Piggin wrote:
> [Fixed linux-mm address. Please reply here]
> 
> Hi,
> 
> I'd like opinions on this patch (applies on top of the previous
> page_mkwrite fixes in -mm). I was not going to ask to merge it
> immediately however it appears that fsblock is not the only one who
> needs it...
> --
> 
> I want to have the page be protected by page lock between page_mkwrite
> notification to the filesystem, and the actual setting of the page
> dirty. Do this by allowing the filesystem to return a locked page from
> page_mkwrite, and have the page fault code keep it held until after it
> calls set_page_dirty.
> 
> I need this in fsblock because I am working to ensure filesystem metadata
> can be correctly allocated and refcounted. This means that page cleaning
> should not require memory allocation.
> 
> Without this patch, then for example we could get a concurrent writeout
> after the page_mkwrite (which allocates page metadata required to clean
> it), but before the set_page_dirty. The writeout will clean the page and
> notice that the metadata is now unused and may be deallocated (because
> it appears clean as set_page_dirty hasn't been called yet). So at this
> point the page may be dirtied via the pte without enough metadata to be
> able to write it back.
> 
> Sage needs this race closed for ceph, and Trond maybe for NFS.

I ran a few tests and this fixes the problem for me (although fyi the 
patch didn't apply cleanly on top of your previously posted page_mkwrite 
prototype change patch, due to some differences in block_page_mkwrite).

Thanks-
sage



> 
> Cc: Sage Weil <sage@newdream.net>
> Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
> Signed-off-by: Nick Piggin <npiggin@suse.de>
> 
> ---
>  Documentation/filesystems/Locking |   24 +++++++---
>  fs/buffer.c                       |   10 ++--
>  mm/memory.c                       |   83 ++++++++++++++++++++++++++------------
>  3 files changed, 79 insertions(+), 38 deletions(-)
> 
> Index: linux-2.6/fs/buffer.c
> ===================================================================
> --- linux-2.6.orig/fs/buffer.c
> +++ linux-2.6/fs/buffer.c
> @@ -2480,7 +2480,8 @@ block_page_mkwrite(struct vm_area_struct
>  	if ((page->mapping != inode->i_mapping) ||
>  	    (page_offset(page) > size)) {
>  		/* page got truncated out from underneath us */
> -		goto out_unlock;
> +		unlock_page(page);
> +		goto out;
>  	}
>  
>  	/* page is wholly or partially inside EOF */
> @@ -2494,14 +2495,15 @@ block_page_mkwrite(struct vm_area_struct
>  		ret = block_commit_write(page, 0, end);
>  
>  	if (unlikely(ret)) {
> +		unlock_page(page);
>  		if (ret == -ENOMEM)
>  			ret = VM_FAULT_OOM;
>  		else /* -ENOSPC, -EIO, etc */
>  			ret = VM_FAULT_SIGBUS;
> -	}
> +	} else
> +		ret = VM_FAULT_LOCKED;
>  
> -out_unlock:
> -	unlock_page(page);
> +out:
>  	return ret;
>  }
>  
> Index: linux-2.6/mm/memory.c
> ===================================================================
> --- linux-2.6.orig/mm/memory.c
> +++ linux-2.6/mm/memory.c
> @@ -1964,6 +1964,15 @@ static int do_wp_page(struct mm_struct *
>  				ret = tmp;
>  				goto unwritable_page;
>  			}
> +			if (unlikely(!(tmp & VM_FAULT_LOCKED))) {
> +				lock_page(old_page);
> +				if (!old_page->mapping) {
> +					ret = 0; /* retry the fault */
> +					unlock_page(old_page);
> +					goto unwritable_page;
> +				}
> +			} else
> +				VM_BUG_ON(!PageLocked(old_page));
>  
>  			/*
>  			 * Since we dropped the lock we need to revalidate
> @@ -1973,9 +1982,11 @@ static int do_wp_page(struct mm_struct *
>  			 */
>  			page_table = pte_offset_map_lock(mm, pmd, address,
>  							 &ptl);
> -			page_cache_release(old_page);
> -			if (!pte_same(*page_table, orig_pte))
> +			if (!pte_same(*page_table, orig_pte)) {
> +				page_cache_release(old_page);
> +				unlock_page(old_page);
>  				goto unlock;
> +			}
>  
>  			page_mkwrite = 1;
>  		}
> @@ -2098,16 +2109,30 @@ unlock:
>  		 *
>  		 * do_no_page is protected similarly.
>  		 */
> -		wait_on_page_locked(dirty_page);
> -		set_page_dirty_balance(dirty_page, page_mkwrite);
> +		if (!page_mkwrite) {
> +			wait_on_page_locked(dirty_page);
> +			set_page_dirty_balance(dirty_page, page_mkwrite);
> +		}
>  		put_page(dirty_page);
> +		if (page_mkwrite) {
> +			struct address_space *mapping = old_page->mapping;
> +
> +			unlock_page(old_page);
> +			page_cache_release(old_page);
> +			balance_dirty_pages_ratelimited(mapping);
> +		}
>  	}
>  	return ret;
>  oom_free_new:
>  	page_cache_release(new_page);
>  oom:
> -	if (old_page)
> +	if (old_page) {
> +		if (page_mkwrite) {
> +			unlock_page(old_page);
> +			page_cache_release(old_page);
> +		}
>  		page_cache_release(old_page);
> +	}
>  	return VM_FAULT_OOM;
>  
>  unwritable_page:
> @@ -2659,27 +2684,22 @@ static int __do_fault(struct mm_struct *
>  				int tmp;
>  
>  				unlock_page(page);
> -				vmf.flags |= FAULT_FLAG_MKWRITE;
> +				vmf.flags = FAULT_FLAG_WRITE|FAULT_FLAG_MKWRITE;
>  				tmp = vma->vm_ops->page_mkwrite(vma, &vmf);
>  				if (unlikely(tmp &
>  					  (VM_FAULT_ERROR | VM_FAULT_NOPAGE))) {
>  					ret = tmp;
> -					anon = 1; /* no anon but release vmf.page */
> -					goto out_unlocked;
> -				}
> -				lock_page(page);
> -				/*
> -				 * XXX: this is not quite right (racy vs
> -				 * invalidate) to unlock and relock the page
> -				 * like this, however a better fix requires
> -				 * reworking page_mkwrite locking API, which
> -				 * is better done later.
> -				 */
> -				if (!page->mapping) {
> -					ret = 0;
> -					anon = 1; /* no anon but release vmf.page */
> -					goto out;
> +					goto unwritable_page;
>  				}
> +				if (unlikely(!(tmp & VM_FAULT_LOCKED))) {
> +					lock_page(page);
> +					if (!page->mapping) {
> +						ret = 0; /* retry the fault */
> +						unlock_page(page);
> +						goto unwritable_page;
> +					}
> +				} else
> +					VM_BUG_ON(!PageLocked(page));
>  				page_mkwrite = 1;
>  			}
>  		}
> @@ -2731,19 +2751,30 @@ static int __do_fault(struct mm_struct *
>  	pte_unmap_unlock(page_table, ptl);
>  
>  out:
> -	unlock_page(vmf.page);
> -out_unlocked:
> -	if (anon)
> -		page_cache_release(vmf.page);
> -	else if (dirty_page) {
> +	if (dirty_page) {
> +		struct address_space *mapping = page->mapping;
> +
>  		if (vma->vm_file)
>  			file_update_time(vma->vm_file);
>  
> +		if (set_page_dirty(dirty_page))
> +			page_mkwrite = 1;
>  		set_page_dirty_balance(dirty_page, page_mkwrite);
> +		unlock_page(dirty_page);
>  		put_page(dirty_page);
> +		if (page_mkwrite)
> +			balance_dirty_pages_ratelimited(mapping);
> +	} else {
> +		unlock_page(vmf.page);
> +		if (anon)
> +			page_cache_release(vmf.page);
>  	}
>  
>  	return ret;
> +
> +unwritable_page:
> +	page_cache_release(page);
> +	return ret;
>  }
>  
>  static int do_linear_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> Index: linux-2.6/Documentation/filesystems/Locking
> ===================================================================
> --- linux-2.6.orig/Documentation/filesystems/Locking
> +++ linux-2.6/Documentation/filesystems/Locking
> @@ -509,16 +509,24 @@ locking rules:
>  		BKL	mmap_sem	PageLocked(page)
>  open:		no	yes
>  close:		no	yes
> -fault:		no	yes
> -page_mkwrite:	no	yes		no
> +fault:		no	yes		can return with page locked
> +page_mkwrite:	no	yes		can return with page locked
>  access:		no	yes
>  
> -	->page_mkwrite() is called when a previously read-only page is
> -about to become writeable. The file system is responsible for
> -protecting against truncate races. Once appropriate action has been
> -taking to lock out truncate, the page range should be verified to be
> -within i_size. The page mapping should also be checked that it is not
> -NULL.
> +	->fault() is called when a previously not present pte is about
> +to be faulted in. The filesystem must find and return the page associated
> +with the passed in "pgoff" in the vm_fault structure. If it is possible that
> +the page may be truncated and/or invalidated, then the filesystem must lock
> +the page, then ensure it is not already truncated (the page lock will block
> +subsequent truncate), and then return with VM_FAULT_LOCKED, and the page
> +locked. The VM will unlock the page.
> +
> +	->page_mkwrite() is called when a previously read-only pte is
> +about to become writeable. The filesystem again must ensure that there are
> +no truncate/invalidate races, and then return with the page locked. If
> +the page has been truncated, the filesystem should not look up a new page
> +like the ->fault() handler, but simply return with VM_FAULT_NOPAGE, which
> +will cause the VM to retry the fault.
>  
>  	->access() is called when get_user_pages() fails in
>  acces_process_vm(), typically used to debug a process through
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

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

Re: [patch] mm: close page_mkwrite races

[Andrew Morton]

On Tue, 31 Mar 2009 12:55:16 -0700 (PDT)
Sage Weil <sage@newdream.net> wrote:

> On Mon, 30 Mar 2009, Nick Piggin wrote:
> > [Fixed linux-mm address. Please reply here]
> > 
> > Hi,
> > 
> > I'd like opinions on this patch (applies on top of the previous
> > page_mkwrite fixes in -mm). I was not going to ask to merge it
> > immediately however it appears that fsblock is not the only one who
> > needs it...
> > --
> > 
> > I want to have the page be protected by page lock between page_mkwrite
> > notification to the filesystem, and the actual setting of the page
> > dirty. Do this by allowing the filesystem to return a locked page from
> > page_mkwrite, and have the page fault code keep it held until after it
> > calls set_page_dirty.
> > 
> > I need this in fsblock because I am working to ensure filesystem metadata
> > can be correctly allocated and refcounted. This means that page cleaning
> > should not require memory allocation.

wot?  "page cleaning" involves writeout.  How can we avoid doing
allocations there?

> > Without this patch, then for example we could get a concurrent writeout
> > after the page_mkwrite (which allocates page metadata required to clean
> > it), but before the set_page_dirty. The writeout will clean the page and
> > notice that the metadata is now unused and may be deallocated (because
> > it appears clean as set_page_dirty hasn't been called yet). So at this
> > point the page may be dirtied via the pte without enough metadata to be
> > able to write it back.
> > 
> > Sage needs this race closed for ceph, and Trond maybe for NFS.
> 
> I ran a few tests and this fixes the problem for me (although fyi the 
> patch didn't apply cleanly on top of your previously posted page_mkwrite 
> prototype change patch, due to some differences in block_page_mkwrite).

What is "the problem"?  Can we get "the problem"'s description included
in the changelog?

The patch is fairly ugly, somewhat costly and makes things (even) more
complex.    Sigh.

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

Re: [patch] mm: close page_mkwrite races

[Trond Myklebust]

On Wed, 2009-04-01 at 16:02 -0700, Andrew Morton wrote:
> What is "the problem"?  Can we get "the problem"'s description included
> in the changelog?
> 
> The patch is fairly ugly, somewhat costly and makes things (even) more
> complex.    Sigh.

The problem is that currently, pages can be marked as dirty after they
have been written out, or even during writeout.

IOW: the filesystem and the mm no longer agree on the state of the page,
which again triggers issues such as
  http://bugzilla.kernel.org/show_bug.cgi?id=12913

Trond

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

Re: [patch] mm: close page_mkwrite races

[Nick Piggin]

On Wed, Apr 01, 2009 at 07:33:31PM -0400, Trond Myklebust wrote:
> On Wed, 2009-04-01 at 16:02 -0700, Andrew Morton wrote:
> > What is "the problem"?  Can we get "the problem"'s description included
> > in the changelog?
> > 
> > The patch is fairly ugly, somewhat costly and makes things (even) more
> > complex.    Sigh.
> 
> The problem is that currently, pages can be marked as dirty after they
> have been written out, or even during writeout.
> 
> IOW: the filesystem and the mm no longer agree on the state of the page,
> which again triggers issues such as
>   http://bugzilla.kernel.org/show_bug.cgi?id=12913

Thanks, that would go nicely at the top of the changelog.


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

Re: [patch] mm: close page_mkwrite races

[Nick Piggin]

On Wed, Apr 01, 2009 at 04:02:41PM -0700, Andrew Morton wrote:
> On Tue, 31 Mar 2009 12:55:16 -0700 (PDT)
> Sage Weil <sage@newdream.net> wrote:
> 
> > On Mon, 30 Mar 2009, Nick Piggin wrote:
> > > [Fixed linux-mm address. Please reply here]
> > > 
> > > Hi,
> > > 
> > > I'd like opinions on this patch (applies on top of the previous
> > > page_mkwrite fixes in -mm). I was not going to ask to merge it
> > > immediately however it appears that fsblock is not the only one who
> > > needs it...
> > > --
> > > 
> > > I want to have the page be protected by page lock between page_mkwrite
> > > notification to the filesystem, and the actual setting of the page
> > > dirty. Do this by allowing the filesystem to return a locked page from
> > > page_mkwrite, and have the page fault code keep it held until after it
> > > calls set_page_dirty.
> > > 
> > > I need this in fsblock because I am working to ensure filesystem metadata
> > > can be correctly allocated and refcounted. This means that page cleaning
> > > should not require memory allocation.
> 
> wot?  "page cleaning" involves writeout.  How can we avoid doing
> allocations there?

Oh, guaranteed allocations like from mempools are fine of course.
But yeah the changelog is slightly more fsblock oriented than it
probably needs to be... I didn't really edit it after taking out
of my series.


> > > Without this patch, then for example we could get a concurrent writeout
> > > after the page_mkwrite (which allocates page metadata required to clean
> > > it), but before the set_page_dirty. The writeout will clean the page and
> > > notice that the metadata is now unused and may be deallocated (because
> > > it appears clean as set_page_dirty hasn't been called yet). So at this
> > > point the page may be dirtied via the pte without enough metadata to be
> > > able to write it back.
> > > 
> > > Sage needs this race closed for ceph, and Trond maybe for NFS.
> > 
> > I ran a few tests and this fixes the problem for me (although fyi the 
> > patch didn't apply cleanly on top of your previously posted page_mkwrite 
> > prototype change patch, due to some differences in block_page_mkwrite).
> 
> What is "the problem"?  Can we get "the problem"'s description included
> in the changelog?
> 
> The patch is fairly ugly, somewhat costly and makes things (even) more
> complex.    Sigh.

I guess it is another step toward being able to pull page_mkwrite into
->fault which will avoid one lock/unlock cycle and lots of stuff in
the main fault path.
 
Also fixes bug of course.


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