Re: [PATCH] btrfs: change the set_page_extent_mapped() call into an ASSERT()
From: Qu Wenruo <hidden>
Date: 2021-07-27 10:29:25
On 2021/7/27 上午9:39, Qu Wenruo wrote:
Btrfs uses set_page_extent_mapped() to properly setup a page. That function would set PagePrivate, and populate needed structure for subpage. The timing of calling set_page_extent_mapped() happens before reading a page or dirtying a page. Thus when we got a page to write back, if it doesn't have PagePrivate, it is a big problem in code logic. Calling set_page_extent_mapped() for such page would just mask the problem. Furthermore, for subpage case, we call subpage error helper to clear the page error bit before calling set_page_extent_mapped(). If we really got a page without Private bit, it can call kernel NULL pointer dereference. So change the set_page_extent_mapped() call to an ASSERT(), and move the check before any page status update call. Signed-off-by: Qu Wenruo <redacted>
Please discard the patch. Although I haven't hit any problem testing the patch, it's still possible that we have a special page that would need cow fixup. Such page will be: - Dirty - Not Private Thus no page->private, this could still cause problem for subpage case though - No EXTENT_DELALLOC flags set for any range inside the page Thus writepage_delalloc() will not find a delalloc range inside the page. Such page will be caught by btrfs_writepage_cow_fixup(), but it will trigger the ASSERT() added by this patch. Thanks, Qu
quoted hunk ↗ jump to hunk
--- fs/btrfs/extent_io.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 62f0ed2de2b9..219add264acf 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c@@ -4099,6 +4099,12 @@ static int __extent_writepage(struct page *page, struct writeback_control *wbc, WARN_ON(!PageLocked(page)); + /* + * All dirty page to be written should have PagePrivate set by + * set_page_extent_mapped() when creating the page. + */ + ASSERT(PagePrivate(page)); + btrfs_page_clear_error(btrfs_sb(inode->i_sb), page, page_offset(page), PAGE_SIZE);@@ -4115,12 +4121,6 @@ static int __extent_writepage(struct page *page, struct writeback_control *wbc, flush_dcache_page(page); } - ret = set_page_extent_mapped(page); - if (ret < 0) { - SetPageError(page); - goto done; - } - if (!epd->extent_locked) { ret = writepage_delalloc(BTRFS_I(inode), page, wbc, &nr_written);