diff options
| author | Filipe Manana <fdmanana@suse.com> | 2016-01-27 13:37:47 -0500 |
|---|---|---|
| committer | Filipe Manana <fdmanana@suse.com> | 2016-02-03 14:27:10 -0500 |
| commit | 313140023026ae542ad76e7e268c56a1eaa2c28e (patch) | |
| tree | 9c9d929f8c7a820659ab2b9e6c88cc1e0a0896b9 /fs | |
| parent | e0bd70c67bf996b360f706b6c643000f2e384681 (diff) | |
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 f441460202cb ("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: f441460202cb ("btrfs: fix deadlock with extent-same and readpage")
Cc: stable@vger.kernel.org # 4.2+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Diffstat (limited to 'fs')
| -rw-r--r-- | fs/btrfs/ioctl.c | 29 |
1 files changed, 21 insertions, 8 deletions
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 1d6767c4c092..561aa6292c1e 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c | |||
| @@ -2794,21 +2794,27 @@ out: | |||
| 2794 | static struct page *extent_same_get_page(struct inode *inode, pgoff_t index) | 2794 | static struct page *extent_same_get_page(struct inode *inode, pgoff_t index) |
| 2795 | { | 2795 | { |
| 2796 | struct page *page; | 2796 | struct page *page; |
| 2797 | struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree; | ||
| 2798 | 2797 | ||
| 2799 | page = grab_cache_page(inode->i_mapping, index); | 2798 | page = grab_cache_page(inode->i_mapping, index); |
| 2800 | if (!page) | 2799 | if (!page) |
| 2801 | return NULL; | 2800 | return ERR_PTR(-ENOMEM); |
| 2802 | 2801 | ||
| 2803 | if (!PageUptodate(page)) { | 2802 | if (!PageUptodate(page)) { |
| 2804 | if (extent_read_full_page_nolock(tree, page, btrfs_get_extent, | 2803 | int ret; |
| 2805 | 0)) | 2804 | |
| 2806 | return NULL; | 2805 | ret = btrfs_readpage(NULL, page); |
| 2806 | if (ret) | ||
| 2807 | return ERR_PTR(ret); | ||
| 2807 | lock_page(page); | 2808 | lock_page(page); |
| 2808 | if (!PageUptodate(page)) { | 2809 | if (!PageUptodate(page)) { |
| 2809 | unlock_page(page); | 2810 | unlock_page(page); |
| 2810 | page_cache_release(page); | 2811 | 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); | ||
| 2812 | } | 2818 | } |
| 2813 | } | 2819 | } |
| 2814 | 2820 | ||
| @@ -2822,9 +2828,16 @@ static int gather_extent_pages(struct inode *inode, struct page **pages, | |||
| 2822 | pgoff_t index = off >> PAGE_CACHE_SHIFT; | 2828 | pgoff_t index = off >> PAGE_CACHE_SHIFT; |
| 2823 | 2829 | ||
| 2824 | for (i = 0; i < num_pages; i++) { | 2830 | for (i = 0; i < num_pages; i++) { |
| 2831 | again: | ||
| 2825 | pages[i] = extent_same_get_page(inode, index + i); | 2832 | pages[i] = extent_same_get_page(inode, index + i); |
| 2826 | if (!pages[i]) | 2833 | if (IS_ERR(pages[i])) { |
| 2827 | return -ENOMEM; | 2834 | int err = PTR_ERR(pages[i]); |
| 2835 | |||
| 2836 | if (err == -EAGAIN) | ||
| 2837 | goto again; | ||
| 2838 | pages[i] = NULL; | ||
| 2839 | return err; | ||
| 2840 | } | ||
| 2828 | } | 2841 | } |
| 2829 | return 0; | 2842 | return 0; |
| 2830 | } | 2843 | } |