| Message ID | 1490382822-25344-1-git-send-email-bo.li.liu@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Mar 24, 2017 at 12:13:42PM -0700, Liu Bo wrote: > In raid56 senario, after trying parity recovery, we didn't set > mirror_num for btrfs_bio with failed mirror_num, hence > end_bio_extent_readpage() will report a random mirror_num in dmesg > log. > > Cc: David Sterba <dsterba@suse.cz> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > --- > fs/btrfs/volumes.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 73d56ee..be64e4a 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -6197,6 +6197,7 @@ int btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > } else { > ret = raid56_parity_recover(fs_info, bio, bbio, > map_length, mirror_num, 1); > + btrfs_io_bio(bio)->mirror_num = mirror_num; Should the mirror be set inside raid56_parity_recover? There's another caller, scrub_submit_raid56_bio_wait, that does not update the bio->mirror_num. I am not sure if this is the same case though. > } > > btrfs_bio_counter_dec(fs_info); -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 27, 2017 at 07:07:15PM +0200, David Sterba wrote: > On Fri, Mar 24, 2017 at 12:13:42PM -0700, Liu Bo wrote: > > In raid56 senario, after trying parity recovery, we didn't set > > mirror_num for btrfs_bio with failed mirror_num, hence > > end_bio_extent_readpage() will report a random mirror_num in dmesg > > log. > > > > Cc: David Sterba <dsterba@suse.cz> > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > > --- > > fs/btrfs/volumes.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > > index 73d56ee..be64e4a 100644 > > --- a/fs/btrfs/volumes.c > > +++ b/fs/btrfs/volumes.c > > @@ -6197,6 +6197,7 @@ int btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > > } else { > > ret = raid56_parity_recover(fs_info, bio, bbio, > > map_length, mirror_num, 1); > > + btrfs_io_bio(bio)->mirror_num = mirror_num; > > Should the mirror be set inside raid56_parity_recover? There's another > caller, scrub_submit_raid56_bio_wait, that does not update the > bio->mirror_num. I am not sure if this is the same case though. > scrub_submit_raid56_bio_wait doesn't need to, but yes, we could set the mirror inside raid56_parity_recover iff @generic_io is 1. Thanks, -liubo > > } > > > > btrfs_bio_counter_dec(fs_info); -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 73d56ee..be64e4a 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -6197,6 +6197,7 @@ int btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, } else { ret = raid56_parity_recover(fs_info, bio, bbio, map_length, mirror_num, 1); + btrfs_io_bio(bio)->mirror_num = mirror_num; } btrfs_bio_counter_dec(fs_info);
In raid56 senario, after trying parity recovery, we didn't set mirror_num for btrfs_bio with failed mirror_num, hence end_bio_extent_readpage() will report a random mirror_num in dmesg log. Cc: David Sterba <dsterba@suse.cz> Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- fs/btrfs/volumes.c | 1 + 1 file changed, 1 insertion(+)