Re: [PATCH v7 03/13] btrfs: introduce a new helper to submit read bio for scrub

[Christoph Hellwig <hch@infradead.org>]

On Thu, Mar 30, 2023 at 02:43:02PM +0800, Qu Wenruo wrote:
> > > 
> > > The repair part would be the same as non-RAID56, as we only need to try
> > > the next mirror.
> > 
> > Didn't we just agree that we do not need another magic helper?
> 
> I have changed the code in github repo so the read path just goes
> btrfs_submit_bio().
> 
> The patch adding bbio::fs_info and adds all the extra skips if bbio->inode
> is NULL:
> https://github.com/adam900710/linux/commit/23c5a1bf8ce98f205de574a15a0eb0518e56e80c

This seems to be a loose object in the repository.  I looked at
d8c93c7078fa8d75bd3e12f8a30df072831f1c47 on the scrub_stripe branch
instead.

This looks generally good to me.  A bunch of really minor nits:

In btrfs_check_read_bio the comment for the assert reads a little
strange to me, maybe

	/* Read-repair requires the inode field to be set by the submitter. */

In btrfs_end_bio_work I'd rework the check to check the inode first,
and add brances around the & check and test positively, i.e.

	if (bbio->inode && !(bbio->bio.bi_opf & REQ_META))
		btrfs_check_read_bio()
	else
		bbio->end_io(bbio);

as that reads easier.  It also matches what btrfs_raid56_end_io does.

In btrfs_submit_bio I'd add an empty line between the assert and
the while loop.

In alloc_new_bio I'd keep the setor initialization before the inode
and file_offset.