[PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[Xin Zhong]

This problem is found in meego testing:
http://bugs.meego.com/show_bug.cgi?id=6672
A file in btrfs is mmaped and the mmaped buffer is passed to pwrite to write to the same page
of the same file. In btrfs_file_aio_write(), the pages is locked by prepare_pages(). So when
btrfs_copy_from_user() is called, page fault happens and the same page needs to be locked again
in filemap_fault(). The fix is to move iov_iter_fault_in_readable() before prepage_pages() to make page
fault happen before pages are locked. And also disable page fault in critical region in
btrfs_copy_from_user().

Signed-off-by: Xin Zhong <xin.zhong@intel.com>
---
 fs/btrfs/file.c |   23 +++++------------------
 1 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index dfe15dc..c1faded 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -57,15 +57,11 @@ static noinline int btrfs_copy_from_user(loff_t pos, int num_pages,
 				     PAGE_CACHE_SIZE - offset, write_bytes);
 		struct page *page = prepared_pages[pg];
 again:
-		/* 
-		 * Copy data from userspace to the current page 
-		 *
-		 * Disable pagefault to avoid recursive lock since the pages 
-		 * are already locked
-		 */
-		pagefault_disable();
-		copied = iov_iter_copy_from_user_atomic(page, i, offset, count);
-		pagefault_enable();
+		if (unlikely(iov_iter_fault_in_readable(i, count)))
+			return -EFAULT;
+
+		/* Copy data from userspace to the current page */
+		copied = iov_iter_copy_from_user(page, i, offset, count);
 
 		/* Flush processor's dcache for this page */
 		flush_dcache_page(page);
@@ -978,15 +974,6 @@ static ssize_t btrfs_file_aio_write(struct kiocb *iocb,
 		if (ret)
 			goto out;
 
-		/* 
-		 * fault pages before locking them in prepare_pages 
-		 * to avoid recursive lock 
-                 */
-		if (unlikely(iov_iter_fault_in_readable(&i, write_bytes))) {
-			ret = -EFAULT;
-			goto out;
-		}
-
 		ret = prepare_pages(root, file, pages, num_pages,
 				    pos, first_index, last_index,
 				    write_bytes);
-- 
1.6.2.2

RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[Zhong, Xin]

The other filesystems such as ext3 do not have this problem since they are using generic_file_buffered_write(). And this problem is fixed back in 2007 for generic_file_buffered_write():
http://lkml.org/lkml/2007/2/4/26

I am very new to btrfs. I am wondering why btrfs has it's own write routine?
Thanks!

-----Original Message-----
From: Zhong, Xin 
Sent: Tuesday, December 07, 2010 4:00 PM
To: linux-btrfs@vger.kernel.org
Cc: Zhong, Xin
Subject: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

This problem is found in meego testing:
http://bugs.meego.com/show_bug.cgi?id=6672
A file in btrfs is mmaped and the mmaped buffer is passed to pwrite to write to the same page
of the same file. In btrfs_file_aio_write(), the pages is locked by prepare_pages(). So when
btrfs_copy_from_user() is called, page fault happens and the same page needs to be locked again
in filemap_fault(). The fix is to move iov_iter_fault_in_readable() before prepage_pages() to make page
fault happen before pages are locked. And also disable page fault in critical region in
btrfs_copy_from_user().

Signed-off-by: Xin Zhong <xin.zhong@intel.com>
---
 fs/btrfs/file.c |   23 +++++------------------
 1 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index dfe15dc..c1faded 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -57,15 +57,11 @@ static noinline int btrfs_copy_from_user(loff_t pos, int num_pages,
 				     PAGE_CACHE_SIZE - offset, write_bytes);
 		struct page *page = prepared_pages[pg];
 again:
-		/* 
-		 * Copy data from userspace to the current page 
-		 *
-		 * Disable pagefault to avoid recursive lock since the pages 
-		 * are already locked
-		 */
-		pagefault_disable();
-		copied = iov_iter_copy_from_user_atomic(page, i, offset, count);
-		pagefault_enable();
+		if (unlikely(iov_iter_fault_in_readable(i, count)))
+			return -EFAULT;
+
+		/* Copy data from userspace to the current page */
+		copied = iov_iter_copy_from_user(page, i, offset, count);
 
 		/* Flush processor's dcache for this page */
 		flush_dcache_page(page);
@@ -978,15 +974,6 @@ static ssize_t btrfs_file_aio_write(struct kiocb *iocb,
 		if (ret)
 			goto out;
 
-		/* 
-		 * fault pages before locking them in prepare_pages 
-		 * to avoid recursive lock 
-                 */
-		if (unlikely(iov_iter_fault_in_readable(&i, write_bytes))) {
-			ret = -EFAULT;
-			goto out;
-		}
-
 		ret = prepare_pages(root, file, pages, num_pages,
 				    pos, first_index, last_index,
 				    write_bytes);
-- 
1.6.2.2

RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[Zhong, Xin]

Sorry, the format of the patch is not correct. I will submit again.

-----Original Message-----
From: linux-btrfs-owner@vger.kernel.org [mailto:linux-btrfs-owner@vger.kernel.org] On Behalf Of Zhong, Xin
Sent: Tuesday, December 07, 2010 4:46 PM
To: linux-btrfs@vger.kernel.org
Subject: RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

The other filesystems such as ext3 do not have this problem since they are using generic_file_buffered_write(). And this problem is fixed back in 2007 for generic_file_buffered_write():
http://lkml.org/lkml/2007/2/4/26

I am very new to btrfs. I am wondering why btrfs has it's own write routine?
Thanks!

-----Original Message-----
From: Zhong, Xin 
Sent: Tuesday, December 07, 2010 4:00 PM
To: linux-btrfs@vger.kernel.org
Cc: Zhong, Xin
Subject: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

This problem is found in meego testing:
http://bugs.meego.com/show_bug.cgi?id=6672
A file in btrfs is mmaped and the mmaped buffer is passed to pwrite to write to the same page
of the same file. In btrfs_file_aio_write(), the pages is locked by prepare_pages(). So when
btrfs_copy_from_user() is called, page fault happens and the same page needs to be locked again
in filemap_fault(). The fix is to move iov_iter_fault_in_readable() before prepage_pages() to make page
fault happen before pages are locked. And also disable page fault in critical region in
btrfs_copy_from_user().

Signed-off-by: Xin Zhong <xin.zhong@intel.com>
---
 fs/btrfs/file.c |   23 +++++------------------
 1 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index dfe15dc..c1faded 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -57,15 +57,11 @@ static noinline int btrfs_copy_from_user(loff_t pos, int num_pages,
 				     PAGE_CACHE_SIZE - offset, write_bytes);
 		struct page *page = prepared_pages[pg];
 again:
-		/* 
-		 * Copy data from userspace to the current page 
-		 *
-		 * Disable pagefault to avoid recursive lock since the pages 
-		 * are already locked
-		 */
-		pagefault_disable();
-		copied = iov_iter_copy_from_user_atomic(page, i, offset, count);
-		pagefault_enable();
+		if (unlikely(iov_iter_fault_in_readable(i, count)))
+			return -EFAULT;
+
+		/* Copy data from userspace to the current page */
+		copied = iov_iter_copy_from_user(page, i, offset, count);
 
 		/* Flush processor's dcache for this page */
 		flush_dcache_page(page);
@@ -978,15 +974,6 @@ static ssize_t btrfs_file_aio_write(struct kiocb *iocb,
 		if (ret)
 			goto out;
 
-		/* 
-		 * fault pages before locking them in prepare_pages 
-		 * to avoid recursive lock 
-                 */
-		if (unlikely(iov_iter_fault_in_readable(&i, write_bytes))) {
-			ret = -EFAULT;
-			goto out;
-		}
-
 		ret = prepare_pages(root, file, pages, num_pages,
 				    pos, first_index, last_index,
 				    write_bytes);
-- 
1.6.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[Xin Zhong]

This problem is found in meego testing:
http://bugs.meego.com/show_bug.cgi?id=6672
A file in btrfs is mmaped and the mmaped buffer is passed to pwrite to write to the same page
of the same file. In btrfs_file_aio_write(), the pages is locked by prepare_pages(). So when
btrfs_copy_from_user() is called, page fault happens and the same page needs to be locked again
in filemap_fault(). The fix is to move iov_iter_fault_in_readable() before prepage_pages() to make page
fault happen before pages are locked. And also disable page fault in critical region in
btrfs_copy_from_user().

Signed-off-by: Xin Zhong <xin.zhong@intel.com>
---
 fs/btrfs/file.c |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index c1faded..805f2ee 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -57,11 +57,15 @@ static noinline int btrfs_copy_from_user(loff_t pos, int num_pages,
 				     PAGE_CACHE_SIZE - offset, write_bytes);
 		struct page *page = prepared_pages[pg];
 again:
-		if (unlikely(iov_iter_fault_in_readable(i, count)))
-			return -EFAULT;
-
-		/* Copy data from userspace to the current page */
-		copied = iov_iter_copy_from_user(page, i, offset, count);
+		/*
+		 * Copy data from userspace to the current page
+		 *
+		 * Disable pagefault to avoid recursive lock since
+		 * the pages are already locked
+		 */
+		pagefault_disable();
+		copied = iov_iter_copy_from_user_atomic(page, i, offset, count);
+		pagefault_enable();

 		/* Flush processor's dcache for this page */
 		flush_dcache_page(page);
@@ -974,6 +978,15 @@ static ssize_t btrfs_file_aio_write(struct kiocb *iocb,
 		if (ret)
 			goto out;

+		/*
+		 * fault pages before locking them in prepare_pages
+		 * to avoid recursive lock
+		 */
+		if (unlikely(iov_iter_fault_in_readable(&i, write_bytes))) {
+			ret = -EFAULT;
+			goto out;
+		}
+
 		ret = prepare_pages(root, file, pages, num_pages,
 				    pos, first_index, last_index,
 				    write_bytes);
--
1.6.2.2

RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[Zhong, Xin]

The previous patch is not ok (http://www.spinics.net/lists/linux-btrfs/msg07447.html).  Please ignore it. Thanks!

-----Original Message-----
From: Zhong, Xin 
Sent: Tuesday, December 07, 2010 5:25 PM
To: linux-btrfs@vger.kernel.org
Cc: Zhong, Xin
Subject: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

This problem is found in meego testing:
http://bugs.meego.com/show_bug.cgi?id=6672
A file in btrfs is mmaped and the mmaped buffer is passed to pwrite to write to the same page
of the same file. In btrfs_file_aio_write(), the pages is locked by prepare_pages(). So when
btrfs_copy_from_user() is called, page fault happens and the same page needs to be locked again
in filemap_fault(). The fix is to move iov_iter_fault_in_readable() before prepage_pages() to make page
fault happen before pages are locked. And also disable page fault in critical region in
btrfs_copy_from_user().

Signed-off-by: Xin Zhong <xin.zhong@intel.com>
---
 fs/btrfs/file.c |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index c1faded..805f2ee 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -57,11 +57,15 @@ static noinline int btrfs_copy_from_user(loff_t pos, int num_pages,
 				     PAGE_CACHE_SIZE - offset, write_bytes);
 		struct page *page = prepared_pages[pg];
 again:
-		if (unlikely(iov_iter_fault_in_readable(i, count)))
-			return -EFAULT;
-
-		/* Copy data from userspace to the current page */
-		copied = iov_iter_copy_from_user(page, i, offset, count);
+		/*
+		 * Copy data from userspace to the current page
+		 *
+		 * Disable pagefault to avoid recursive lock since
+		 * the pages are already locked
+		 */
+		pagefault_disable();
+		copied = iov_iter_copy_from_user_atomic(page, i, offset, count);
+		pagefault_enable();

 		/* Flush processor's dcache for this page */
 		flush_dcache_page(page);
@@ -974,6 +978,15 @@ static ssize_t btrfs_file_aio_write(struct kiocb *iocb,
 		if (ret)
 			goto out;

+		/*
+		 * fault pages before locking them in prepare_pages
+		 * to avoid recursive lock
+		 */
+		if (unlikely(iov_iter_fault_in_readable(&i, write_bytes))) {
+			ret = -EFAULT;
+			goto out;
+		}
+
 		ret = prepare_pages(root, file, pages, num_pages,
 				    pos, first_index, last_index,
 				    write_bytes);
--
1.6.2.2

RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[Zhong, Xin]

The other filesystems such as ext3 do not have this problem since they are using generic_file_buffered_write(). And this problem is fixed back in 2007 for generic_file_buffered_write():
http://lkml.org/lkml/2007/2/4/26

I am very new to btrfs. I am wondering why btrfs has it's own write routine?
Thanks!

-----Original Message-----
From: Zhong, Xin 
Sent: Tuesday, December 07, 2010 5:25 PM
To: linux-btrfs@vger.kernel.org
Cc: Zhong, Xin
Subject: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

This problem is found in meego testing:
http://bugs.meego.com/show_bug.cgi?id=6672
A file in btrfs is mmaped and the mmaped buffer is passed to pwrite to write to the same page
of the same file. In btrfs_file_aio_write(), the pages is locked by prepare_pages(). So when
btrfs_copy_from_user() is called, page fault happens and the same page needs to be locked again
in filemap_fault(). The fix is to move iov_iter_fault_in_readable() before prepage_pages() to make page
fault happen before pages are locked. And also disable page fault in critical region in
btrfs_copy_from_user().

Signed-off-by: Xin Zhong <xin.zhong@intel.com>
---
 fs/btrfs/file.c |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index c1faded..805f2ee 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -57,11 +57,15 @@ static noinline int btrfs_copy_from_user(loff_t pos, int num_pages,
 				     PAGE_CACHE_SIZE - offset, write_bytes);
 		struct page *page = prepared_pages[pg];
 again:
-		if (unlikely(iov_iter_fault_in_readable(i, count)))
-			return -EFAULT;
-
-		/* Copy data from userspace to the current page */
-		copied = iov_iter_copy_from_user(page, i, offset, count);
+		/*
+		 * Copy data from userspace to the current page
+		 *
+		 * Disable pagefault to avoid recursive lock since
+		 * the pages are already locked
+		 */
+		pagefault_disable();
+		copied = iov_iter_copy_from_user_atomic(page, i, offset, count);
+		pagefault_enable();

 		/* Flush processor's dcache for this page */
 		flush_dcache_page(page);
@@ -974,6 +978,15 @@ static ssize_t btrfs_file_aio_write(struct kiocb *iocb,
 		if (ret)
 			goto out;

+		/*
+		 * fault pages before locking them in prepare_pages
+		 * to avoid recursive lock
+		 */
+		if (unlikely(iov_iter_fault_in_readable(&i, write_bytes))) {
+			ret = -EFAULT;
+			goto out;
+		}
+
 		ret = prepare_pages(root, file, pages, num_pages,
 				    pos, first_index, last_index,
 				    write_bytes);
--
1.6.2.2

Re: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[Christoph Hellwig]

Care to write a xfstests test for this behaviour?

RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[Zhong, Xin]

I have sent my test case to xfs mailing list (No. 248). 
Please help to review it. Thanks!

-----Original Message-----
From: Christoph Hellwig [mailto:hch@infradead.org] 
Sent: Wednesday, December 08, 2010 12:55 AM
To: Zhong, Xin
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

Care to write a xfstests test for this behaviour?

RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[Chris Mason]

Excerpts from Zhong, Xin's message of 2010-12-13 03:26:04 -0500:
> I have sent my test case to xfs mailing list (No. 248). 
> Please help to review it. Thanks!

You btrfs patch is now in the master branch of the btrfs unstable tree.
Thanks!

-chris

RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page

[Zhong, Xin]

Q2hyaXMsIHRoYW5rcyBhIGxvdCBmb3IgeW91ciBxdWljayByZXNwb25zZS4NCg0KVGhhbmtzIHRv
IFpoZW5nIFlhbiB0b28gZm9yIGhpcyBjYXJlZnVsIHJldmlldy4NCg0KLS0tLS1PcmlnaW5hbCBN
ZXNzYWdlLS0tLS0NCkZyb206IENocmlzIE1hc29uIFttYWlsdG86Y2hyaXMubWFzb25Ab3JhY2xl
LmNvbV0gDQpTZW50OiBUdWVzZGF5LCBEZWNlbWJlciAxNCwgMjAxMCA5OjM2IEFNDQpUbzogWmhv
bmcsIFhpbg0KQ2M6IENocmlzdG9waCBIZWxsd2lnOyBsaW51eC1idHJmc0B2Z2VyLmtlcm5lbC5v
cmcNClN1YmplY3Q6IFJFOiBbUEFUQ0hdIEJ0cmZzOiBwd3JpdGUgYmxvY2tlZCB3aGVuIHdyaXRp
bmcgZnJvbSB0aGUgbW1hcGVkIGJ1ZmZlciBvZiB0aGUgc2FtZSBwYWdlDQoNCkV4Y2VycHRzIGZy
b20gWmhvbmcsIFhpbidzIG1lc3NhZ2Ugb2YgMjAxMC0xMi0xMyAwMzoyNjowNCAtMDUwMDoNCj4g
SSBoYXZlIHNlbnQgbXkgdGVzdCBjYXNlIHRvIHhmcyBtYWlsaW5nIGxpc3QgKE5vLiAyNDgpLiAN
Cj4gUGxlYXNlIGhlbHAgdG8gcmV2aWV3IGl0LiBUaGFua3MhDQoNCllvdSBidHJmcyBwYXRjaCBp
cyBub3cgaW4gdGhlIG1hc3RlciBicmFuY2ggb2YgdGhlIGJ0cmZzIHVuc3RhYmxlIHRyZWUuDQpU
aGFua3MhDQoNCi1jaHJpcw0K