btrfs: return all mirror whether need_raid_map set or not
cover btrfs: return all mirror whether need_raid_map set or not
__btrfs_map_block() should return all mirror on WRITE, REQ_GET_READ_MIRRORS, and RECOVERY case, whether need_raid_map set or not.
need_raid_map only used to control is to set bbio->raid_map.
Current code works right becuase there is only one caller can trigger above bug, which is readahead, and this function happened to bypass on less mirror.
But after we fixed __btrfs_map_block(), readahead will be really works, and exit with warning at another bug.
This patchset fixed __btrfs_map_block(), and disable raid56 readahead temp temporary, (actually, it already disable by this bug), and I'll fix raid56 readahead next.
Zhao Lei (4):
btrfs: Disable raid56 readahead
btrfs: return all mirror whether need_raid_map set or not
btrfs: Small cleanup for get index_srcdev loop
btrfs: Use direct way to determine raid56 write/recover mode
fs/btrfs/reada.c | 5 +++++ fs/btrfs/volumes.c | 50 ++++++++++++++++++++++++-------------------------- 2 files changed, 29 insertions(+), 26 deletions(-)
Signature · 1 line
1.8.5.1
PATCH 1/4 btrfs: Disable raid56 readahead
Raid56 readahead can not work in current code, reada_find_extent() will show warning of bbio->num_stripes > BTRFS_MAX_MIRRORS, because raid56 have parity strip, which makes more bbio->num_stripes.
The reason why we haven't see above error is because another bug in __btrfs_map_block(), which make raid56 readahead do nothing.
Before we will fix bug in __btrfs_map_block(), we need to disable raid56 temporary, to avoid above warning.
--- fs/btrfs/reada.c | 5 +++++ 1 file changed, 5 insertions(+)
fs/btrfs/reada.c +5 -0
@@ -363,6 +363,11 @@ static struct reada_extent *reada_find_extent(struct btrfs_root *root,if (ret || !bbio || length < blocksize) goto error; +if (bbio->map_type &BTRFS_BLOCK_GROUP_RAID56_MASK ) { +/* Current code can not support RAID56 yet */ + goto error; + } +if (bbio->num_stripes >BTRFS_MAX_MIRRORS ) { btrfs_err(root->fs_info, "readahead: more than %d copies not supported",
Signature · 1 line
1.8.5.1
PATCH 2/4 btrfs: return all mirror whether need_raid_map set or not
In reply to Zhao Lei__btrfs_map_block() should return all mirror on WRITE, REQ_GET_READ_MIRRORS, and RECOVERY case, whether need_raid_map set or not.
need_raid_map only used to control is to set bbio->raid_map.
Current code works right because there is only one caller can trigger above bug, which is readahead, and this function happened to bypass on less mirror.
--- fs/btrfs/volumes.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
fs/btrfs/volumes.c +2 -3
@@ -5464,9 +5464,8 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, int rw, } }else if (map->type &BTRFS_BLOCK_GROUP_RAID56_MASK ) { -if (need_raid_map && - ((rw & (REQ_WRITE |REQ_GET_READ_MIRRORS )) || - mirror_num >1 )) { +if ((rw & (REQ_WRITE |REQ_GET_READ_MIRRORS )) || + mirror_num >1 ) {/* push stripe_nr back to the start of the full stripe */ stripe_nr =div_u64 (raid56_full_stripe_start, stripe_len *nr_data_stripes (map));
Signature · 1 line
1.8.5.1
PATCH 3/4 btrfs: Small cleanup for get index_srcdev loop
In reply to Zhao Lei1: Adjust condition in loop to make less TAB
2: Move btrfs_put_bbio()'s line for combine, and makes logic clean.
--- fs/btrfs/volumes.c | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-)
fs/btrfs/volumes.c +20 -22
@@ -5368,35 +5368,33 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, int rw,* target drive. */ for (i =0 ; i < tmp_num_stripes; i++) { -if (tmp_bbio->stripes[i].dev->devid == srcdev_devid) { -/* -* In case of DUP, in order to keep it -* simple, only add the mirror with the -* lowest physical address -*/ -if (found && - physical_of_found <= - tmp_bbio->stripes[i].physical) -continue ; - index_srcdev = i; - found =1 ; - physical_of_found = - tmp_bbio->stripes[i].physical; - } +if (tmp_bbio->stripes[i].dev->devid != srcdev_devid) +continue ; + +/* +* In case of DUP, in order to keep it simple, only add +* the mirror with the lowest physical address +*/ +if (found && + physical_of_found <= tmp_bbio->stripes[i].physical) +continue ; + + index_srcdev = i; + found =1 ; + physical_of_found = tmp_bbio->stripes[i].physical; } -if (found) { - mirror_num = index_srcdev +1 ; - patch_the_first_stripe_for_dev_replace =1 ; - physical_to_patch_in_first_stripe = physical_of_found; - }else { +btrfs_put_bbio (tmp_bbio); + +if (!found) {WARN_ON (1 ); ret = -EIO ; -btrfs_put_bbio (tmp_bbio); goto out; } -btrfs_put_bbio (tmp_bbio); + mirror_num = index_srcdev +1 ; + patch_the_first_stripe_for_dev_replace =1 ; + physical_to_patch_in_first_stripe = physical_of_found; }else if (mirror_num > map->num_stripes) { mirror_num =0 ; }
Signature · 1 line
1.8.5.1
PATCH 4/4 btrfs: Use direct way to determine raid56 write/recover mode
In reply to Zhao LeiOld code use bbio->raid_map to determine whether in raid56 write/recover operation, because we don't have bbio->map_type that time, and have to use above workaround.
Now we have direct way for this condition, to get gid of using the function-relative data, and make code readable.
--- fs/btrfs/volumes.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
fs/btrfs/volumes.c +2 -1
@@ -6056,7 +6056,8 @@ int btrfs_map_bio(struct btrfs_root *root, int rw, struct bio *bio, bbio->fs_info = root->fs_info;atomic_set (&bbio->stripes_pending, bbio->num_stripes); -if (bbio->raid_map) { +if ((bbio->map_type &BTRFS_BLOCK_GROUP_RAID56_MASK ) && + ((rw &WRITE ) || (mirror_num >1 ))) {/* In this case, map_length has been set to the length of a single stripe; not the whole write */ if (rw &WRITE ) {
Signature · 1 line
1.8.5.1
Quoted 16 lines · Zhao Lei
__btrfs_map_block()should return all mirror on WRITE,REQ_GET_READ_MIRRORS, and RECOVERY case, whetherneed_raid_mapset or not.
need_raid_maponly used to control is to setbbio->raid_map.Current code works right becuase there is only one caller can trigger above bug, which is readahead, and this function happened to bypass on less mirror.
But after we fixed
__btrfs_map_block(), readahead will be really works, and exit with warning at another bug.
This patchset fixed__btrfs_map_block(), and disable raid56 readahead temp temporary, (actually, it already disable by this bug), and I'll fix raid56 readahead next.
Quoted 5 lines
Zhao Lei (4):
btrfs: Disable raid56 readahead
btrfs: return all mirror whetherneed_raid_mapset or not
btrfs: Small cleanup for getindex_srcdevloop
btrfs: Use direct way to determine raid56 write/recover mode
Are these patches still relevant? If yes, please update and resed, thanks.
Quoted 17 lines · David Sterba
Quoted 16 lines · Zhao Lei
__btrfs_map_block()should return all mirror on WRITE,REQ_GET_READ_MIRRORS, and RECOVERY case, whetherneed_raid_mapset or not.
need_raid_maponly used to control is to setbbio->raid_map.Current code works right becuase there is only one caller can trigger above bug, which is readahead, and this function happened to bypass on less mirror.
But after we fixed
__btrfs_map_block(), readahead will be really works, and exit with warning at another bug.
This patchset fixed__btrfs_map_block(), and disable raid56 readahead temp temporary, (actually, it already disable by this bug), and I'll fix raid56 readahead next.
Quoted 8 lines
Quoted 5 lines
Zhao Lei (4):
btrfs: Disable raid56 readahead
btrfs: return all mirror whetherneed_raid_mapset or not
btrfs: Small cleanup for getindex_srcdevloop
btrfs: Use direct way to determine raid56 write/recover modeAre these patches still relevant? If yes, please update and resed, thanks.
Hi David,
Zhao doesn't work on btrfs any more.
So I'll re-check these patches and see if they are still needed. If so, I'll rebase (some of) them and resend.
Thanks,
Qu
Quoted 6 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html