[V5,12/13] Btrfs: prepare_pages: Retry adding a page to the page cache
diff mbox

Message ID 1443608912-31667-13-git-send-email-chandan@linux.vnet.ibm.com
State New
Headers show

Commit Message

Chandan Rajendra Sept. 30, 2015, 10:28 a.m. UTC
When reading the page from the disk, we can race with Direct I/O which can get
the page lock (before prepare_uptodate_page() gets it) and can go ahead and
invalidate the page. Hence if the page is not found in the inode's address
space, retry the operation of getting a page.

Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
---
 fs/btrfs/file.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Josef Bacik Oct. 1, 2015, 2:50 p.m. UTC | #1
On 09/30/2015 06:28 AM, Chandan Rajendra wrote:
> When reading the page from the disk, we can race with Direct I/O which can get
> the page lock (before prepare_uptodate_page() gets it) and can go ahead and
> invalidate the page. Hence if the page is not found in the inode's address
> space, retry the operation of getting a page.
>
> Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> ---

Huh, how in the world did you make that happen?

Reviewed-by: Josef Bacik <jbacik@fb.com>

Thanks,

Josef
--
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
Chandan Rajendra Oct. 2, 2015, 12:24 p.m. UTC | #2
On Thursday 01 Oct 2015 10:50:30 Josef Bacik wrote:
> On 09/30/2015 06:28 AM, Chandan Rajendra wrote:
> > When reading the page from the disk, we can race with Direct I/O which can
> > get the page lock (before prepare_uptodate_page() gets it) and can go
> > ahead and invalidate the page. Hence if the page is not found in the
> > inode's address space, retry the operation of getting a page.
> > 
> > Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> > ---
> 
> Huh, how in the world did you make that happen?
>

The issue is seen when generic/095 test is run in a loop. I would like to add,

Reported-by: Jakub Palider <jpa@semihalf.com>

> Reviewed-by: Josef Bacik <jbacik@fb.com>
> 
> Thanks,
> 
> Josef

Patch
diff mbox

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 5715e29..76db77c 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1316,6 +1316,7 @@  static noinline int prepare_pages(struct inode *inode, struct page **pages,
 	int faili;
 
 	for (i = 0; i < num_pages; i++) {
+again:
 		pages[i] = find_or_create_page(inode->i_mapping, index + i,
 					       mask | __GFP_WRITE);
 		if (!pages[i]) {
@@ -1330,6 +1331,21 @@  static noinline int prepare_pages(struct inode *inode, struct page **pages,
 		if (i == num_pages - 1)
 			err = prepare_uptodate_page(pages[i],
 						    pos + write_bytes, false);
+
+		/*
+		 * When reading the page from the disk, we can race
+		 * with direct i/o which can get the page lock (before
+		 * prepare_uptodate_page() gets it) and can go ahead
+		 * and invalidate the page. Hence if the page is found
+		 * to be not belonging to the inode's address space,
+		 * retry the operation of getting a page.
+		 */
+		if (unlikely(pages[i]->mapping != inode->i_mapping)) {
+			unlock_page(pages[i]);
+			page_cache_release(pages[i]);
+			goto again;
+		}
+
 		if (err) {
 			page_cache_release(pages[i]);
 			faili = i - 1;