[npiggin@wotan.suse.de: Re: [patch 00/41] Buffered write deadlock fix and new aops for 2.6.21-mm2]

[Nick Piggin <npiggin@suse.de>]

I actually forgot to cc linux-fsdevel on this one.

Vladimir found a corner case bug in the case of faulting source
address, which is since fixed, but might be interesting to anyone
else following development...

----- Forwarded message from Nick Piggin <npiggin@wotan.suse.de> -----

Date: Wed, 16 May 2007 09:14:06 +0200
From: Nick Piggin <npiggin@wotan.suse.de>
To: "Vladimir V. Saveliev" <vs@namesys.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [patch 00/41] Buffered write deadlock fix and new aops for 2.6.21-mm2
In-Reply-To: <200705152200.38257.vs@namesys.com>
User-Agent: Mutt/1.5.9i

cc'ed linux-fsdevel again...

On Tue, May 15, 2007 at 10:00:38PM +0400, Vladimir V. Saveliev wrote:
> Hello
> 
> On Tuesday 15 May 2007 02:42, Nick Piggin wrote:
> > On Mon, May 14, 2007 at 10:28:45PM +0400, Vladimir V. Saveliev wrote:
> > > Hello
> > > 
> > > There is a problem with new write.
> > > 
> > > If you expand an empty file with truncate and then write so that in one write file tail  is overwritten and something is appended to the file - the write loops forever
> > > writing to page containing file tail. Something wrong happens writing to uptodate last page of a file, I guess.
> > > 
> > > I can send a simple program if necessary.
> > 
> > Is this reiserfs with your reiserfs patch? 
> 
> No, this is common problem. I get it easily on ext3 and ext2.
> 
> > Yes, please send a simple 
> > program and I'll have a look.

Thanks, that was really helpful!

What the program does is to write a non-faulted page into pagecache,
which means we're relying on fault_in_pages_readable to bring it in
for us. Usually it does, however your specific write pattern required
that 2 pages be brought in, and _also_ that the total number of bytes
to write went past that 2nd page.

This caused the 1st and an Nth (>2) page to be faulted in, but the
atomic copy_from_user really needed the 2nd page :)

This fixes it for me.
---

Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2007-05-16 16:58:41.000000000 +1000
+++ linux-2.6/include/linux/fs.h	2007-05-16 16:58:47.000000000 +1000
@@ -419,7 +419,7 @@
 size_t iov_iter_copy_from_user(struct page *page,
 		struct iov_iter *i, unsigned long offset, size_t bytes);
 void iov_iter_advance(struct iov_iter *i, size_t bytes);
-int iov_iter_fault_in_readable(struct iov_iter *i);
+int iov_iter_fault_in_readable(struct iov_iter *i, size_t bytes);
 size_t iov_iter_single_seg_count(struct iov_iter *i);
 
 static inline void iov_iter_init(struct iov_iter *i,
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c	2007-05-16 14:11:18.000000000 +1000
+++ linux-2.6/mm/filemap.c	2007-05-16 17:03:29.000000000 +1000
@@ -1806,11 +1806,10 @@
 }
 EXPORT_SYMBOL(iov_iter_advance);
 
-int iov_iter_fault_in_readable(struct iov_iter *i)
+int iov_iter_fault_in_readable(struct iov_iter *i, size_t bytes)
 {
-	size_t seglen = min(i->iov->iov_len - i->iov_offset, i->count);
 	char __user *buf = i->iov->iov_base + i->iov_offset;
-	return fault_in_pages_readable(buf, seglen);
+	return fault_in_pages_readable(buf, bytes);
 }
 EXPORT_SYMBOL(iov_iter_fault_in_readable);
 
@@ -2102,7 +2101,7 @@
 		 * to check that the address is actually valid, when atomic
 		 * usercopies are used, below.
 		 */
-		if (unlikely(iov_iter_fault_in_readable(i))) {
+		if (unlikely(iov_iter_fault_in_readable(i, bytes))) {
 			status = -EFAULT;
 			break;
 		}
@@ -2276,7 +2275,7 @@
 		 * to check that the address is actually valid, when atomic
 		 * usercopies are used, below.
 		 */
-		if (unlikely(iov_iter_fault_in_readable(i))) {
+		if (unlikely(iov_iter_fault_in_readable(i, bytes))) {
 			status = -EFAULT;
 			break;
 		}

----- End forwarded message -----