Re: [PATCH v7 01/17] btrfs: properly reset @this_bio_flag in btrfs_do_readpage() to avoid inheriting old bio flags to next extent

[Anand Jain <anand.jain@oracle.com>]

On 12/07/2021 16:30, Qu Wenruo wrote:
> In btrfs_do_readpage(), we never reset @this_bio_flag after we hit a
> compressed extent.
> 
> This is fine, as for PAGE_SIZE == sectorsize case, we can only have one
> sector for one page, thus @this_bio_flag will only be set at most once.
> 
> But for subpage case, after hitting a compressed extent, @this_bio_flag
> will always have EXTENT_BIO_COMPRESSED bit, even we're reading a regular
> extent.
> 
> This will lead to various read error, and causing new ASSERT() in
> incoming subpage patches, which adds more strict check in
> btrfs_submit_compressed_read().
> 
> Fix it by declaring @this_bio_flag inside the main loop and reset its
> value for each iteration.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

looks good.
Reviewed-by: Anand Jain <anand.jain@oracle.com>

> ---
>   fs/btrfs/extent_io.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 1f947e24091a..a834ba61a729 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -3487,7 +3487,6 @@ int btrfs_do_readpage(struct page *page, struct extent_map **em_cached,
>   	size_t pg_offset = 0;
>   	size_t iosize;
>   	size_t blocksize = inode->i_sb->s_blocksize;
> -	unsigned long this_bio_flag = 0;
>   	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
>   
>   	ret = set_page_extent_mapped(page);
> @@ -3518,6 +3517,7 @@ int btrfs_do_readpage(struct page *page, struct extent_map **em_cached,
>   	}
>   	begin_page_read(fs_info, page);
>   	while (cur <= end) {
> +		unsigned long this_bio_flag = 0;
>   		bool force_bio_submit = false;
>   		u64 disk_bytenr;
>   
>