| Message ID | 8c38c2f9d1bee46ded4ec491e16398f2f5764200.1637745470.git.johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: first batch of zoned cleanups | expand |
On Wed, Nov 24, 2021 at 01:30:29AM -0800, Johannes Thumshirn wrote: > Sink zone check into btrfs_repair_one_zone() so we don't need to do it in > all callers. > > Also as btrfs_repair_one_zone() doesn't return a sensible error, just > make it a void function. > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > --- > fs/btrfs/extent_io.c | 3 +-- > fs/btrfs/scrub.c | 4 ++-- > fs/btrfs/volumes.c | 13 ++++++++----- > fs/btrfs/volumes.h | 2 +- > 4 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 1654c611d2002..96c2e40887772 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -2314,8 +2314,7 @@ static int repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start, > ASSERT(!(fs_info->sb->s_flags & SB_RDONLY)); > BUG_ON(!mirror_num); > > - if (btrfs_is_zoned(fs_info)) > - return btrfs_repair_one_zone(fs_info, logical); > + btrfs_repair_one_zone(fs_info, logical); > > bio = btrfs_bio_alloc(1); > bio->bi_iter.bi_size = 0; > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index d175c5ab11349..30bb304bb5e9a 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -852,8 +852,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check) > have_csum = sblock_to_check->pagev[0]->have_csum; > dev = sblock_to_check->pagev[0]->dev; > > - if (btrfs_is_zoned(fs_info) && !sctx->is_dev_replace) > - return btrfs_repair_one_zone(fs_info, logical); > + if (!sctx->is_dev_replace) > + btrfs_repair_one_zone(fs_info, logical); > > /* > * We must use GFP_NOFS because the scrub task might be waiting for a > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index ae1a4f2a8af64..d0615256dacc3 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -8329,23 +8329,26 @@ static int relocating_repair_kthread(void *data) > return ret; > } > > -int btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical) > +void btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical) > { > struct btrfs_block_group *cache; > > + if (!btrfs_is_zoned(fs_info)) > + return; > + > /* Do not attempt to repair in degraded state */ > if (btrfs_test_opt(fs_info, DEGRADED)) > - return 0; > + return; > > cache = btrfs_lookup_block_group(fs_info, logical); > if (!cache) > - return 0; > + return; > > spin_lock(&cache->lock); > if (cache->relocating_repair) { > spin_unlock(&cache->lock); > btrfs_put_block_group(cache); > - return 0; > + return; > } > cache->relocating_repair = 1; > spin_unlock(&cache->lock); > @@ -8353,5 +8356,5 @@ int btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical) > kthread_run(relocating_repair_kthread, cache, > "btrfs-relocating-repair"); > > - return 0; > + return; This is extraneous. Thanks, Josef
On Wed, Nov 24, 2021 at 01:30:29AM -0800, Johannes Thumshirn wrote: > Sink zone check into btrfs_repair_one_zone() so we don't need to do it in > all callers. > > Also as btrfs_repair_one_zone() doesn't return a sensible error, just > make it a void function. > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > --- > fs/btrfs/extent_io.c | 3 +-- > fs/btrfs/scrub.c | 4 ++-- > fs/btrfs/volumes.c | 13 ++++++++----- > fs/btrfs/volumes.h | 2 +- > 4 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 1654c611d2002..96c2e40887772 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -2314,8 +2314,7 @@ static int repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start, > ASSERT(!(fs_info->sb->s_flags & SB_RDONLY)); > BUG_ON(!mirror_num); > > - if (btrfs_is_zoned(fs_info)) > - return btrfs_repair_one_zone(fs_info, logical); > + btrfs_repair_one_zone(fs_info, logical); This changes the flow, is that intended? Previously in zoned mode the function would end here and won't go down to allocate bio etc, now it would. > bio = btrfs_bio_alloc(1); > bio->bi_iter.bi_size = 0;
On 24/11/2021 18:30, David Sterba wrote: > On Wed, Nov 24, 2021 at 01:30:29AM -0800, Johannes Thumshirn wrote: >> Sink zone check into btrfs_repair_one_zone() so we don't need to do it in >> all callers. >> >> Also as btrfs_repair_one_zone() doesn't return a sensible error, just >> make it a void function. >> >> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> >> --- >> fs/btrfs/extent_io.c | 3 +-- >> fs/btrfs/scrub.c | 4 ++-- >> fs/btrfs/volumes.c | 13 ++++++++----- >> fs/btrfs/volumes.h | 2 +- >> 4 files changed, 12 insertions(+), 10 deletions(-) >> >> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >> index 1654c611d2002..96c2e40887772 100644 >> --- a/fs/btrfs/extent_io.c >> +++ b/fs/btrfs/extent_io.c >> @@ -2314,8 +2314,7 @@ static int repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start, >> ASSERT(!(fs_info->sb->s_flags & SB_RDONLY)); >> BUG_ON(!mirror_num); >> >> - if (btrfs_is_zoned(fs_info)) >> - return btrfs_repair_one_zone(fs_info, logical); >> + btrfs_repair_one_zone(fs_info, logical); > > This changes the flow, is that intended? Previously in zoned mode the > function would end here and won't go down to allocate bio etc, now it > would. Oops nope that's unintentional and a bug. Thanks for catching it.
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 1654c611d2002..96c2e40887772 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -2314,8 +2314,7 @@ static int repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start, ASSERT(!(fs_info->sb->s_flags & SB_RDONLY)); BUG_ON(!mirror_num); - if (btrfs_is_zoned(fs_info)) - return btrfs_repair_one_zone(fs_info, logical); + btrfs_repair_one_zone(fs_info, logical); bio = btrfs_bio_alloc(1); bio->bi_iter.bi_size = 0; diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index d175c5ab11349..30bb304bb5e9a 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -852,8 +852,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check) have_csum = sblock_to_check->pagev[0]->have_csum; dev = sblock_to_check->pagev[0]->dev; - if (btrfs_is_zoned(fs_info) && !sctx->is_dev_replace) - return btrfs_repair_one_zone(fs_info, logical); + if (!sctx->is_dev_replace) + btrfs_repair_one_zone(fs_info, logical); /* * We must use GFP_NOFS because the scrub task might be waiting for a diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index ae1a4f2a8af64..d0615256dacc3 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -8329,23 +8329,26 @@ static int relocating_repair_kthread(void *data) return ret; } -int btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical) +void btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical) { struct btrfs_block_group *cache; + if (!btrfs_is_zoned(fs_info)) + return; + /* Do not attempt to repair in degraded state */ if (btrfs_test_opt(fs_info, DEGRADED)) - return 0; + return; cache = btrfs_lookup_block_group(fs_info, logical); if (!cache) - return 0; + return; spin_lock(&cache->lock); if (cache->relocating_repair) { spin_unlock(&cache->lock); btrfs_put_block_group(cache); - return 0; + return; } cache->relocating_repair = 1; spin_unlock(&cache->lock); @@ -8353,5 +8356,5 @@ int btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical) kthread_run(relocating_repair_kthread, cache, "btrfs-relocating-repair"); - return 0; + return; } diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 3b81306807493..ab30cf4236fa3 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -637,6 +637,6 @@ enum btrfs_raid_types __attribute_const__ btrfs_bg_flags_to_raid_index(u64 flags int btrfs_bg_type_to_factor(u64 flags); const char *btrfs_bg_type_to_raid_name(u64 flags); int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info); -int btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical); +void btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical); #endif
Sink zone check into btrfs_repair_one_zone() so we don't need to do it in all callers. Also as btrfs_repair_one_zone() doesn't return a sensible error, just make it a void function. Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> --- fs/btrfs/extent_io.c | 3 +-- fs/btrfs/scrub.c | 4 ++-- fs/btrfs/volumes.c | 13 ++++++++----- fs/btrfs/volumes.h | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-)