Re: [PATCH 10/16] btrfs: remove non-standard extent handling in __extent_writepage_io

[Christoph Hellwig <hch@lst.de>]

On Sat, Feb 10, 2024 at 08:09:47PM +1030, Qu Wenruo wrote:
>> @@ -1419,10 +1418,14 @@ static noinline_for_stack int __extent_writepage_io(struct btrfs_inode *inode,
>>   		ASSERT(cur < end);
>>   		ASSERT(IS_ALIGNED(em->start, fs_info->sectorsize));
>>   		ASSERT(IS_ALIGNED(em->len, fs_info->sectorsize));
>> +
>>   		block_start = em->block_start;
>> -		compressed = test_bit(EXTENT_FLAG_COMPRESSED, &em->flags);
>>   		disk_bytenr = em->block_start + extent_offset;
>>
>> +		ASSERT(!test_bit(EXTENT_FLAG_COMPRESSED, &em->flags));
>> +		ASSERT(block_start != EXTENT_MAP_HOLE);
>
> For subpage cases, __extent_writepage_io() can be triggered to write
> only a subset of the page, from extent_write_locked_range().

Yes.

> In that case, if we have submitted the target range, since our @len is
> to the end of the page, we can hit a hole.
>
> In that case, this ASSERT() would be triggered.
> And even worse, if CONFIG_BTRFS_ASSERT() is not enabled, we can do wrong
> writeback using the wrong disk_bytenr.
>
> So at least we need to skip the hole ranges for subpage.
> And thankfully the remaining two cases are impossible for subpage.

The patch below reinstates the hole handling.  I don't have a system
that tests the btrfs subpage code right now, so this is untested:

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index cfd2967f04a293..a106036641104c 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1388,7 +1388,6 @@ static noinline_for_stack int __extent_writepage_io(struct btrfs_inode *inode,
 		disk_bytenr = em->block_start + extent_offset;
 
 		ASSERT(!extent_map_is_compressed(em));
-		ASSERT(block_start != EXTENT_MAP_HOLE);
 		ASSERT(block_start != EXTENT_MAP_INLINE);
 
 		/*
@@ -1399,6 +1398,15 @@ static noinline_for_stack int __extent_writepage_io(struct btrfs_inode *inode,
 		free_extent_map(em);
 		em = NULL;
 
+		if (block_start == EXTENT_MAP_HOLE) {
+			btrfs_mark_ordered_io_finished(inode, page, cur, iosize,
+						       true);
+			btrfs_folio_clear_dirty(fs_info, page_folio(page), cur,
+						iosize);
+			cur += iosize;
+			continue;
+		}
+
 		btrfs_set_range_writeback(inode, cur, cur + iosize - 1);
 		if (!PageWriteback(page)) {
 			btrfs_err(inode->root->fs_info,