Re: [PATCH] btrfs: refactor __extent_writepage_io() to do sector-by-sector submission

[David Sterba <dsterba@suse.cz>]

On Thu, Aug 08, 2024 at 07:35:40AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2024/8/8 01:57, David Sterba 写道:
> > On Wed, Aug 07, 2024 at 06:21:00PM +0930, Qu Wenruo wrote:
> [...]
> >
> >> +
> >> +	block_start = extent_map_block_start(em);
> >> +	disk_bytenr = extent_map_block_start(em) + extent_offset;
> >> +
> >> +	ASSERT(!extent_map_is_compressed(em));
> >> +	ASSERT(block_start != EXTENT_MAP_HOLE);
> >> +	ASSERT(block_start != EXTENT_MAP_INLINE);
> >> +
> >> +	free_extent_map(em);
> >> +	em = NULL;
> >> +
> >> +	btrfs_set_range_writeback(inode, filepos, filepos + sectorsize - 1);
> >> +	if (!folio_test_writeback(folio)) {
> >> +		btrfs_err(fs_info,
> >> +			  "folio %lu not writeback, cur %llu end %llu",
> >> +			  folio->index, filepos, filepos + sectorsize);
> >
> > This is copied from the original code but I wonder if this should be a
> > hard error or more visible as it looks like bad page state tracking.
> >
> 
> I believe this can be removed completely, or converted into ASSERT().
> 
> For subpage case, page writeback is set if any subpage sector has
> writeback flag set.
> 
> For regular case, it's setting the whole page writeback.
> 
> As long as the folio is already locked, the folio should always be
> marked writeback.
> 
> So I prefer to go the ASSERT() path.

Agreed, ASSERT makes sense.