Xin Zhong
2010-Dec-07 07:59 UTC
[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
Zhong, Xin
2010-Dec-07 08:46 UTC
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
Zhong, Xin
2010-Dec-07 09:23 UTC
RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page
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 -- 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
Xin Zhong
2010-Dec-07 09:25 UTC
[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 -- 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
Zhong, Xin
2010-Dec-07 09:34 UTC
RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page
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 -- 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
Zhong, Xin
2010-Dec-07 09:34 UTC
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 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 -- 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
Christoph Hellwig
2010-Dec-07 16:54 UTC
Re: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page
Care to write a xfstests test for this behaviour? -- 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
Zhong, Xin
2010-Dec-13 08:26 UTC
RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page
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? -- 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
Chris Mason
2010-Dec-14 01:35 UTC
RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page
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 -- 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
Zhong, Xin
2010-Dec-14 02:44 UTC
RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page
Chris, thanks a lot for your quick response. Thanks to Zheng Yan too for his careful review. -----Original Message----- From: Chris Mason [mailto:chris.mason@oracle.com] Sent: Tuesday, December 14, 2010 9:36 AM To: Zhong, Xin Cc: Christoph Hellwig; linux-btrfs@vger.kernel.org Subject: RE: [PATCH] Btrfs: pwrite blocked when writing from the mmaped buffer of the same page 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 NrybXǧv^){.n+{n߲)w*jgݢj/zޖ2ޙ&)ߡaGhj:+vw٥
Apparently Analagous Threads
- Reproducible kernel BUG while using VirtualBox:
- [PATCH 1/5] fs: allow short direct-io reads to be completed via buffered IO V2
- Re: [Bug 26242] New: BUG: unable to handle kernel NULL pointer dereference at (null)
- null pointer dereference in iov_iter_copy_from_user_atomic while updating rpm packages
- btrfs hang in flush-btrfs-5