Skip to content

Commit 3131400

Browse files
committed
Btrfs: fix page reading in extent_same ioctl leading to csum errors
In the extent_same ioctl, we were grabbing the pages (locked) and attempting to read them without bothering about any concurrent IO against them. That is, we were not checking for any ongoing ordered extents nor waiting for them to complete, which leads to a race where the extent_same() code gets a checksum verification error when it reads the pages, producing a message like the following in dmesg and making the operation fail to user space with -ENOMEM: [18990.161265] BTRFS warning (device sdc): csum failed ino 259 off 495616 csum 685204116 expected csum 1515870868 Fix this by using btrfs_readpage() for reading the pages instead of extent_read_full_page_nolock(), which waits for any concurrent ordered extents to complete and locks the io range. Also do better error handling and don't treat all failures as -ENOMEM, as that's clearly misleasing, becoming identical to the checks and operation of prepare_uptodate_page(). The use of extent_read_full_page_nolock() was required before commit f441460 ("btrfs: fix deadlock with extent-same and readpage"), as we had the range locked in an inode's io tree before attempting to read the pages. Fixes: f441460 ("btrfs: fix deadlock with extent-same and readpage") Cc: [email protected] # 4.2+ Signed-off-by: Filipe Manana <[email protected]>
1 parent e0bd70c commit 3131400

File tree

1 file changed

+21
-8
lines changed

1 file changed

+21
-8
lines changed

fs/btrfs/ioctl.c

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2794,21 +2794,27 @@ static long btrfs_ioctl_dev_info(struct btrfs_root *root, void __user *arg)
27942794
static struct page *extent_same_get_page(struct inode *inode, pgoff_t index)
27952795
{
27962796
struct page *page;
2797-
struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
27982797

27992798
page = grab_cache_page(inode->i_mapping, index);
28002799
if (!page)
2801-
return NULL;
2800+
return ERR_PTR(-ENOMEM);
28022801

28032802
if (!PageUptodate(page)) {
2804-
if (extent_read_full_page_nolock(tree, page, btrfs_get_extent,
2805-
0))
2806-
return NULL;
2803+
int ret;
2804+
2805+
ret = btrfs_readpage(NULL, page);
2806+
if (ret)
2807+
return ERR_PTR(ret);
28072808
lock_page(page);
28082809
if (!PageUptodate(page)) {
28092810
unlock_page(page);
28102811
page_cache_release(page);
2811-
return NULL;
2812+
return ERR_PTR(-EIO);
2813+
}
2814+
if (page->mapping != inode->i_mapping) {
2815+
unlock_page(page);
2816+
page_cache_release(page);
2817+
return ERR_PTR(-EAGAIN);
28122818
}
28132819
}
28142820

@@ -2822,9 +2828,16 @@ static int gather_extent_pages(struct inode *inode, struct page **pages,
28222828
pgoff_t index = off >> PAGE_CACHE_SHIFT;
28232829

28242830
for (i = 0; i < num_pages; i++) {
2831+
again:
28252832
pages[i] = extent_same_get_page(inode, index + i);
2826-
if (!pages[i])
2827-
return -ENOMEM;
2833+
if (IS_ERR(pages[i])) {
2834+
int err = PTR_ERR(pages[i]);
2835+
2836+
if (err == -EAGAIN)
2837+
goto again;
2838+
pages[i] = NULL;
2839+
return err;
2840+
}
28282841
}
28292842
return 0;
28302843
}

0 commit comments

Comments
 (0)