Re: [PATCH v3 20/26] btrfs: make extent_write_locked_range() to be subpage compatible
From: Josef Bacik <josef@toxicpanda.com>
Date: 2021-10-13 14:50:43
On Mon, Sep 27, 2021 at 03:22:02PM +0800, Qu Wenruo wrote:
quoted hunk ↗ jump to hunk
There are two sites are not subpage compatible yet for extent_write_locked_range(): - How @nr_pages are calculated For subpage we can have the following range with 64K page size: 0 32K 64K 96K 128K | |////|/////| | In that case, although 96K - 32K == 64K, thus it looks like one page is enough, but the range spans across two pages, not one. Fix it by doing proper round_up() and round_down() to calculate @nr_pages. Also add some extra ASSERT()s to ensure the range passed in is already aligned. - How the page end is calculated Currently we just use cur + PAGE_SIZE - 1 to calculate the page end. Which can't handle above range layout, and will trigger ASSERT() in btrfs_writepage_endio_finish_ordered(), as the range is no longer covered by the page range. Fix it by take page end into consideration. Signed-off-by: Qu Wenruo <redacted> --- fs/btrfs/extent_io.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 825917f1b623..f05d8896d1ad 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c@@ -5087,15 +5087,14 @@ int extent_write_locked_range(struct inode *inode, u64 start, u64 end) struct address_space *mapping = inode->i_mapping; struct page *page; u64 cur = start; - unsigned long nr_pages = (end - start + PAGE_SIZE) >> - PAGE_SHIFT; + unsigned long nr_pages; + const u32 sectorsize = btrfs_sb(inode->i_sb)->sectorsize; struct extent_page_data epd = { .bio_ctrl = { 0 }, .extent_locked = 1, .sync_io = 1, }; struct writeback_control wbc_writepages = { - .nr_to_write = nr_pages * 2, .sync_mode = WB_SYNC_ALL, .range_start = start, .range_end = end + 1,@@ -5104,14 +5103,24 @@ int extent_write_locked_range(struct inode *inode, u64 start, u64 end) .no_cgroup_owner = 1, }; + ASSERT(IS_ALIGNED(start, sectorsize) && + IS_ALIGNED(end + 1, sectorsize)); + nr_pages = (round_up(end, PAGE_SIZE) - round_down(start, PAGE_SIZE)) >> + PAGE_SHIFT; + wbc_writepages.nr_to_write = nr_pages * 2; + wbc_attach_fdatawrite_inode(&wbc_writepages, inode); while (cur <= end) { + u64 cur_end = min(round_down(cur, PAGE_SIZE) + PAGE_SIZE - 1, + end); + page = find_get_page(mapping, cur >> PAGE_SHIFT); /* * All pages in the range are locked since * btrfs_run_delalloc_range(), thus there is no way to clear * the page dirty flag. */ + ASSERT(PageLocked(page));
We're tripping this ASSERT() with compression turned on, sorry I didn't notice this but we've been panicing consistently since this was merged, so I've lost a weeks worth of xfstests runs. You can easily reproduce running ./check generic/029 with MOUNT_OPTIONS="-o compress" Thanks, Josef