| Message ID | 348c07314744f4914f5d613c516e9790f8c725b5.1710409033.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: scrub: refine the error messages | expand |
On Thu, Mar 14, 2024 at 9:50 AM Qu Wenruo <wqu@suse.com> wrote: > > The @stripe passed into scrub_stripe_report_errors() either has > stripe->dev and stripe->physical properly populated (regular data > stripes) or stripe->flags would have SCRUB_STRIPE_FLAG_NO_REPORT > (RAID56 P/Q stripes). > > Thus there is no need to go with btrfs_map_block() to get the > dev/physical. > > Just add an extra ASSERT() to make sure we get stripe->dev populated > correctly. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/scrub.c | 59 ++++++------------------------------------------ > 1 file changed, 7 insertions(+), 52 deletions(-) > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index 5e371ffdb37b..277583464371 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -860,10 +860,8 @@ static void scrub_stripe_submit_repair_read(struct scrub_stripe *stripe, > static void scrub_stripe_report_errors(struct scrub_ctx *sctx, > struct scrub_stripe *stripe) > { > - static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL, > - DEFAULT_RATELIMIT_BURST); > struct btrfs_fs_info *fs_info = sctx->fs_info; > - struct btrfs_device *dev = NULL; > + struct btrfs_device *dev = stripe->dev; > u64 stripe_physical = stripe->physical; > int nr_data_sectors = 0; > int nr_meta_sectors = 0; > @@ -874,35 +872,7 @@ static void scrub_stripe_report_errors(struct scrub_ctx *sctx, > if (test_bit(SCRUB_STRIPE_FLAG_NO_REPORT, &stripe->state)) > return; > > - /* > - * Init needed infos for error reporting. > - * > - * Although our scrub_stripe infrastructure is mostly based on btrfs_submit_bio() > - * thus no need for dev/physical, error reporting still needs dev and physical. > - */ > - if (!bitmap_empty(&stripe->init_error_bitmap, stripe->nr_sectors)) { > - u64 mapped_len = fs_info->sectorsize; > - struct btrfs_io_context *bioc = NULL; > - int stripe_index = stripe->mirror_num - 1; > - int ret; > - > - /* For scrub, our mirror_num should always start at 1. */ > - ASSERT(stripe->mirror_num >= 1); > - ret = btrfs_map_block(fs_info, BTRFS_MAP_GET_READ_MIRRORS, > - stripe->logical, &mapped_len, &bioc, > - NULL, NULL); > - /* > - * If we failed, dev will be NULL, and later detailed reports > - * will just be skipped. > - */ > - if (ret < 0) > - goto skip; > - stripe_physical = bioc->stripes[stripe_index].physical; > - dev = bioc->stripes[stripe_index].dev; > - btrfs_put_bioc(bioc); > - } > - > -skip: > + ASSERT(dev); > for_each_set_bit(sector_nr, &stripe->extent_sector_bitmap, stripe->nr_sectors) { > u64 logical = stripe->logical + > (sector_nr << fs_info->sectorsize_bits); > @@ -933,42 +903,27 @@ static void scrub_stripe_report_errors(struct scrub_ctx *sctx, > * output the message of repaired message. > */ > if (repaired) { > - if (dev) { > - btrfs_err_rl_in_rcu(fs_info, > + btrfs_err_rl_in_rcu(fs_info, > "fixed up error at logical %llu on dev %s physical %llu", > logical, btrfs_dev_name(dev), > physical); > - } else { > - btrfs_err_rl_in_rcu(fs_info, > - "fixed up error at logical %llu on mirror %u", > - logical, stripe->mirror_num); > - } > continue; > } > > /* The remaining are all for unrepaired. */ > - if (dev) { > - btrfs_err_rl_in_rcu(fs_info, > + btrfs_err_rl_in_rcu(fs_info, > "unable to fixup (regular) error at logical %llu on dev %s physical %llu", > logical, btrfs_dev_name(dev), > physical); > - } else { > - btrfs_err_rl_in_rcu(fs_info, > - "unable to fixup (regular) error at logical %llu on mirror %u", > - logical, stripe->mirror_num); > - } > > if (test_bit(sector_nr, &stripe->io_error_bitmap)) > - if (__ratelimit(&rs) && dev) > - scrub_print_common_warning("i/o error", dev, > + scrub_print_common_warning("i/o error", dev, > logical, physical); > if (test_bit(sector_nr, &stripe->csum_error_bitmap)) > - if (__ratelimit(&rs) && dev) > - scrub_print_common_warning("checksum error", dev, > + scrub_print_common_warning("checksum error", dev, > logical, physical); > if (test_bit(sector_nr, &stripe->meta_error_bitmap)) > - if (__ratelimit(&rs) && dev) > - scrub_print_common_warning("header error", dev, > + scrub_print_common_warning("header error", dev, Why are we removing the rate limiting? This seems like an unrelated change. Everything else looks ok. Thanks. > logical, physical); > } > > -- > 2.44.0 > >
在 2024/3/15 03:56, Filipe Manana 写道: > On Thu, Mar 14, 2024 at 9:50 AM Qu Wenruo <wqu@suse.com> wrote: >> >> The @stripe passed into scrub_stripe_report_errors() either has >> stripe->dev and stripe->physical properly populated (regular data >> stripes) or stripe->flags would have SCRUB_STRIPE_FLAG_NO_REPORT >> (RAID56 P/Q stripes). >> >> Thus there is no need to go with btrfs_map_block() to get the >> dev/physical. >> >> Just add an extra ASSERT() to make sure we get stripe->dev populated >> correctly. >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> fs/btrfs/scrub.c | 59 ++++++------------------------------------------ >> 1 file changed, 7 insertions(+), 52 deletions(-) >> >> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c >> index 5e371ffdb37b..277583464371 100644 >> --- a/fs/btrfs/scrub.c >> +++ b/fs/btrfs/scrub.c >> @@ -860,10 +860,8 @@ static void scrub_stripe_submit_repair_read(struct scrub_stripe *stripe, >> static void scrub_stripe_report_errors(struct scrub_ctx *sctx, >> struct scrub_stripe *stripe) >> { >> - static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL, >> - DEFAULT_RATELIMIT_BURST); >> struct btrfs_fs_info *fs_info = sctx->fs_info; >> - struct btrfs_device *dev = NULL; >> + struct btrfs_device *dev = stripe->dev; >> u64 stripe_physical = stripe->physical; >> int nr_data_sectors = 0; >> int nr_meta_sectors = 0; >> @@ -874,35 +872,7 @@ static void scrub_stripe_report_errors(struct scrub_ctx *sctx, >> if (test_bit(SCRUB_STRIPE_FLAG_NO_REPORT, &stripe->state)) >> return; >> >> - /* >> - * Init needed infos for error reporting. >> - * >> - * Although our scrub_stripe infrastructure is mostly based on btrfs_submit_bio() >> - * thus no need for dev/physical, error reporting still needs dev and physical. >> - */ >> - if (!bitmap_empty(&stripe->init_error_bitmap, stripe->nr_sectors)) { >> - u64 mapped_len = fs_info->sectorsize; >> - struct btrfs_io_context *bioc = NULL; >> - int stripe_index = stripe->mirror_num - 1; >> - int ret; >> - >> - /* For scrub, our mirror_num should always start at 1. */ >> - ASSERT(stripe->mirror_num >= 1); >> - ret = btrfs_map_block(fs_info, BTRFS_MAP_GET_READ_MIRRORS, >> - stripe->logical, &mapped_len, &bioc, >> - NULL, NULL); >> - /* >> - * If we failed, dev will be NULL, and later detailed reports >> - * will just be skipped. >> - */ >> - if (ret < 0) >> - goto skip; >> - stripe_physical = bioc->stripes[stripe_index].physical; >> - dev = bioc->stripes[stripe_index].dev; >> - btrfs_put_bioc(bioc); >> - } >> - >> -skip: >> + ASSERT(dev); >> for_each_set_bit(sector_nr, &stripe->extent_sector_bitmap, stripe->nr_sectors) { >> u64 logical = stripe->logical + >> (sector_nr << fs_info->sectorsize_bits); >> @@ -933,42 +903,27 @@ static void scrub_stripe_report_errors(struct scrub_ctx *sctx, >> * output the message of repaired message. >> */ >> if (repaired) { >> - if (dev) { >> - btrfs_err_rl_in_rcu(fs_info, >> + btrfs_err_rl_in_rcu(fs_info, >> "fixed up error at logical %llu on dev %s physical %llu", >> logical, btrfs_dev_name(dev), >> physical); >> - } else { >> - btrfs_err_rl_in_rcu(fs_info, >> - "fixed up error at logical %llu on mirror %u", >> - logical, stripe->mirror_num); >> - } >> continue; >> } >> >> /* The remaining are all for unrepaired. */ >> - if (dev) { >> - btrfs_err_rl_in_rcu(fs_info, >> + btrfs_err_rl_in_rcu(fs_info, >> "unable to fixup (regular) error at logical %llu on dev %s physical %llu", >> logical, btrfs_dev_name(dev), >> physical); >> - } else { >> - btrfs_err_rl_in_rcu(fs_info, >> - "unable to fixup (regular) error at logical %llu on mirror %u", >> - logical, stripe->mirror_num); >> - } >> >> if (test_bit(sector_nr, &stripe->io_error_bitmap)) >> - if (__ratelimit(&rs) && dev) >> - scrub_print_common_warning("i/o error", dev, >> + scrub_print_common_warning("i/o error", dev, >> logical, physical); >> if (test_bit(sector_nr, &stripe->csum_error_bitmap)) >> - if (__ratelimit(&rs) && dev) >> - scrub_print_common_warning("checksum error", dev, >> + scrub_print_common_warning("checksum error", dev, >> logical, physical); >> if (test_bit(sector_nr, &stripe->meta_error_bitmap)) >> - if (__ratelimit(&rs) && dev) >> - scrub_print_common_warning("header error", dev, >> + scrub_print_common_warning("header error", dev, > > Why are we removing the rate limiting? > This seems like an unrelated change. Because the ratelimit is not consistent. E.g. the "fixed up error" line is not rated limited by @rs, but by btrfs_err_rl_in_rcu(). And we would have scrub_print_common_warning() to go btrfs_*_rl() helper in the coming patches. Thanks, Qu > > Everything else looks ok. > > Thanks. > >> logical, physical); >> } >> >> -- >> 2.44.0 >> >> >
On Thu, Mar 14, 2024 at 8:28 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote: > > > > 在 2024/3/15 03:56, Filipe Manana 写道: > > On Thu, Mar 14, 2024 at 9:50 AM Qu Wenruo <wqu@suse.com> wrote: > >> > >> The @stripe passed into scrub_stripe_report_errors() either has > >> stripe->dev and stripe->physical properly populated (regular data > >> stripes) or stripe->flags would have SCRUB_STRIPE_FLAG_NO_REPORT > >> (RAID56 P/Q stripes). > >> > >> Thus there is no need to go with btrfs_map_block() to get the > >> dev/physical. > >> > >> Just add an extra ASSERT() to make sure we get stripe->dev populated > >> correctly. > >> > >> Signed-off-by: Qu Wenruo <wqu@suse.com> > >> --- > >> fs/btrfs/scrub.c | 59 ++++++------------------------------------------ > >> 1 file changed, 7 insertions(+), 52 deletions(-) > >> > >> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > >> index 5e371ffdb37b..277583464371 100644 > >> --- a/fs/btrfs/scrub.c > >> +++ b/fs/btrfs/scrub.c > >> @@ -860,10 +860,8 @@ static void scrub_stripe_submit_repair_read(struct scrub_stripe *stripe, > >> static void scrub_stripe_report_errors(struct scrub_ctx *sctx, > >> struct scrub_stripe *stripe) > >> { > >> - static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL, > >> - DEFAULT_RATELIMIT_BURST); > >> struct btrfs_fs_info *fs_info = sctx->fs_info; > >> - struct btrfs_device *dev = NULL; > >> + struct btrfs_device *dev = stripe->dev; > >> u64 stripe_physical = stripe->physical; > >> int nr_data_sectors = 0; > >> int nr_meta_sectors = 0; > >> @@ -874,35 +872,7 @@ static void scrub_stripe_report_errors(struct scrub_ctx *sctx, > >> if (test_bit(SCRUB_STRIPE_FLAG_NO_REPORT, &stripe->state)) > >> return; > >> > >> - /* > >> - * Init needed infos for error reporting. > >> - * > >> - * Although our scrub_stripe infrastructure is mostly based on btrfs_submit_bio() > >> - * thus no need for dev/physical, error reporting still needs dev and physical. > >> - */ > >> - if (!bitmap_empty(&stripe->init_error_bitmap, stripe->nr_sectors)) { > >> - u64 mapped_len = fs_info->sectorsize; > >> - struct btrfs_io_context *bioc = NULL; > >> - int stripe_index = stripe->mirror_num - 1; > >> - int ret; > >> - > >> - /* For scrub, our mirror_num should always start at 1. */ > >> - ASSERT(stripe->mirror_num >= 1); > >> - ret = btrfs_map_block(fs_info, BTRFS_MAP_GET_READ_MIRRORS, > >> - stripe->logical, &mapped_len, &bioc, > >> - NULL, NULL); > >> - /* > >> - * If we failed, dev will be NULL, and later detailed reports > >> - * will just be skipped. > >> - */ > >> - if (ret < 0) > >> - goto skip; > >> - stripe_physical = bioc->stripes[stripe_index].physical; > >> - dev = bioc->stripes[stripe_index].dev; > >> - btrfs_put_bioc(bioc); > >> - } > >> - > >> -skip: > >> + ASSERT(dev); > >> for_each_set_bit(sector_nr, &stripe->extent_sector_bitmap, stripe->nr_sectors) { > >> u64 logical = stripe->logical + > >> (sector_nr << fs_info->sectorsize_bits); > >> @@ -933,42 +903,27 @@ static void scrub_stripe_report_errors(struct scrub_ctx *sctx, > >> * output the message of repaired message. > >> */ > >> if (repaired) { > >> - if (dev) { > >> - btrfs_err_rl_in_rcu(fs_info, > >> + btrfs_err_rl_in_rcu(fs_info, > >> "fixed up error at logical %llu on dev %s physical %llu", > >> logical, btrfs_dev_name(dev), > >> physical); > >> - } else { > >> - btrfs_err_rl_in_rcu(fs_info, > >> - "fixed up error at logical %llu on mirror %u", > >> - logical, stripe->mirror_num); > >> - } > >> continue; > >> } > >> > >> /* The remaining are all for unrepaired. */ > >> - if (dev) { > >> - btrfs_err_rl_in_rcu(fs_info, > >> + btrfs_err_rl_in_rcu(fs_info, > >> "unable to fixup (regular) error at logical %llu on dev %s physical %llu", > >> logical, btrfs_dev_name(dev), > >> physical); > >> - } else { > >> - btrfs_err_rl_in_rcu(fs_info, > >> - "unable to fixup (regular) error at logical %llu on mirror %u", > >> - logical, stripe->mirror_num); > >> - } > >> > >> if (test_bit(sector_nr, &stripe->io_error_bitmap)) > >> - if (__ratelimit(&rs) && dev) > >> - scrub_print_common_warning("i/o error", dev, > >> + scrub_print_common_warning("i/o error", dev, > >> logical, physical); > >> if (test_bit(sector_nr, &stripe->csum_error_bitmap)) > >> - if (__ratelimit(&rs) && dev) > >> - scrub_print_common_warning("checksum error", dev, > >> + scrub_print_common_warning("checksum error", dev, > >> logical, physical); > >> if (test_bit(sector_nr, &stripe->meta_error_bitmap)) > >> - if (__ratelimit(&rs) && dev) > >> - scrub_print_common_warning("header error", dev, > >> + scrub_print_common_warning("header error", dev, > > > > Why are we removing the rate limiting? > > This seems like an unrelated change. > > Because the ratelimit is not consistent. > > E.g. the "fixed up error" line is not rated limited by @rs, but by > btrfs_err_rl_in_rcu(). That I don't understand. What I see is that scrub_print_common_warning() calls btrfs_warn_in_rcu() or btrfs_warn() or scrub_print_warning_inode() (which calls btrfs_warn_in_rcu()). I don't see where btrfs_err_rl_in_rcu() is called. So to me it seems @rs is doing the rate limiting. > > And we would have scrub_print_common_warning() to go btrfs_*_rl() helper > in the coming patches. Ok, but if @rs is indeed doing the rate limiting here, then from an organization point of view, it should be removed in the later patch that makes scrub_print_common_warning() use the btrfs_*_rl() helpers. Thanks. > > Thanks, > Qu > > > > Everything else looks ok. > > > > Thanks. > > > >> logical, physical); > >> } > >> > >> -- > >> 2.44.0 > >> > >> > >
在 2024/3/19 02:46, Filipe Manana 写道: > On Thu, Mar 14, 2024 at 8:28 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote: >> >> >> >> 在 2024/3/15 03:56, Filipe Manana 写道: >>> On Thu, Mar 14, 2024 at 9:50 AM Qu Wenruo <wqu@suse.com> wrote: >>>> >>>> The @stripe passed into scrub_stripe_report_errors() either has >>>> stripe->dev and stripe->physical properly populated (regular data >>>> stripes) or stripe->flags would have SCRUB_STRIPE_FLAG_NO_REPORT >>>> (RAID56 P/Q stripes). >>>> >>>> Thus there is no need to go with btrfs_map_block() to get the >>>> dev/physical. >>>> >>>> Just add an extra ASSERT() to make sure we get stripe->dev populated >>>> correctly. >>>> >>>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>>> --- >>>> fs/btrfs/scrub.c | 59 ++++++------------------------------------------ >>>> 1 file changed, 7 insertions(+), 52 deletions(-) >>>> >>>> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c >>>> index 5e371ffdb37b..277583464371 100644 >>>> --- a/fs/btrfs/scrub.c >>>> +++ b/fs/btrfs/scrub.c >>>> @@ -860,10 +860,8 @@ static void scrub_stripe_submit_repair_read(struct scrub_stripe *stripe, >>>> static void scrub_stripe_report_errors(struct scrub_ctx *sctx, >>>> struct scrub_stripe *stripe) >>>> { >>>> - static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL, >>>> - DEFAULT_RATELIMIT_BURST); >>>> struct btrfs_fs_info *fs_info = sctx->fs_info; >>>> - struct btrfs_device *dev = NULL; >>>> + struct btrfs_device *dev = stripe->dev; >>>> u64 stripe_physical = stripe->physical; >>>> int nr_data_sectors = 0; >>>> int nr_meta_sectors = 0; >>>> @@ -874,35 +872,7 @@ static void scrub_stripe_report_errors(struct scrub_ctx *sctx, >>>> if (test_bit(SCRUB_STRIPE_FLAG_NO_REPORT, &stripe->state)) >>>> return; >>>> >>>> - /* >>>> - * Init needed infos for error reporting. >>>> - * >>>> - * Although our scrub_stripe infrastructure is mostly based on btrfs_submit_bio() >>>> - * thus no need for dev/physical, error reporting still needs dev and physical. >>>> - */ >>>> - if (!bitmap_empty(&stripe->init_error_bitmap, stripe->nr_sectors)) { >>>> - u64 mapped_len = fs_info->sectorsize; >>>> - struct btrfs_io_context *bioc = NULL; >>>> - int stripe_index = stripe->mirror_num - 1; >>>> - int ret; >>>> - >>>> - /* For scrub, our mirror_num should always start at 1. */ >>>> - ASSERT(stripe->mirror_num >= 1); >>>> - ret = btrfs_map_block(fs_info, BTRFS_MAP_GET_READ_MIRRORS, >>>> - stripe->logical, &mapped_len, &bioc, >>>> - NULL, NULL); >>>> - /* >>>> - * If we failed, dev will be NULL, and later detailed reports >>>> - * will just be skipped. >>>> - */ >>>> - if (ret < 0) >>>> - goto skip; >>>> - stripe_physical = bioc->stripes[stripe_index].physical; >>>> - dev = bioc->stripes[stripe_index].dev; >>>> - btrfs_put_bioc(bioc); >>>> - } >>>> - >>>> -skip: >>>> + ASSERT(dev); >>>> for_each_set_bit(sector_nr, &stripe->extent_sector_bitmap, stripe->nr_sectors) { >>>> u64 logical = stripe->logical + >>>> (sector_nr << fs_info->sectorsize_bits); >>>> @@ -933,42 +903,27 @@ static void scrub_stripe_report_errors(struct scrub_ctx *sctx, >>>> * output the message of repaired message. >>>> */ >>>> if (repaired) { >>>> - if (dev) { >>>> - btrfs_err_rl_in_rcu(fs_info, >>>> + btrfs_err_rl_in_rcu(fs_info, >>>> "fixed up error at logical %llu on dev %s physical %llu", >>>> logical, btrfs_dev_name(dev), >>>> physical); >>>> - } else { >>>> - btrfs_err_rl_in_rcu(fs_info, >>>> - "fixed up error at logical %llu on mirror %u", >>>> - logical, stripe->mirror_num); >>>> - } >>>> continue; >>>> } >>>> >>>> /* The remaining are all for unrepaired. */ >>>> - if (dev) { >>>> - btrfs_err_rl_in_rcu(fs_info, >>>> + btrfs_err_rl_in_rcu(fs_info, >>>> "unable to fixup (regular) error at logical %llu on dev %s physical %llu", >>>> logical, btrfs_dev_name(dev), >>>> physical); >>>> - } else { >>>> - btrfs_err_rl_in_rcu(fs_info, >>>> - "unable to fixup (regular) error at logical %llu on mirror %u", >>>> - logical, stripe->mirror_num); >>>> - } >>>> >>>> if (test_bit(sector_nr, &stripe->io_error_bitmap)) >>>> - if (__ratelimit(&rs) && dev) >>>> - scrub_print_common_warning("i/o error", dev, >>>> + scrub_print_common_warning("i/o error", dev, >>>> logical, physical); >>>> if (test_bit(sector_nr, &stripe->csum_error_bitmap)) >>>> - if (__ratelimit(&rs) && dev) >>>> - scrub_print_common_warning("checksum error", dev, >>>> + scrub_print_common_warning("checksum error", dev, >>>> logical, physical); >>>> if (test_bit(sector_nr, &stripe->meta_error_bitmap)) >>>> - if (__ratelimit(&rs) && dev) >>>> - scrub_print_common_warning("header error", dev, >>>> + scrub_print_common_warning("header error", dev, >>> >>> Why are we removing the rate limiting? >>> This seems like an unrelated change. >> >> Because the ratelimit is not consistent. >> >> E.g. the "fixed up error" line is not rated limited by @rs, but by >> btrfs_err_rl_in_rcu(). > > That I don't understand. > > What I see is that scrub_print_common_warning() calls > btrfs_warn_in_rcu() or btrfs_warn() or scrub_print_warning_inode() > (which calls btrfs_warn_in_rcu()). > I don't see where btrfs_err_rl_in_rcu() is called. So to me it seems > @rs is doing the rate limiting. > >> >> And we would have scrub_print_common_warning() to go btrfs_*_rl() helper >> in the coming patches. > > Ok, but if @rs is indeed doing the rate limiting here, then from an > organization point of view, it should be removed in the later patch > that makes scrub_print_common_warning() use the btrfs_*_rl() helpers. OK, I'll rearrange the patches so that @rs would only be removed after we have migrate all the messages are migrated to rate limited versions. Thanks, Qu > > Thanks. > >> >> Thanks, >> Qu >>> >>> Everything else looks ok. >>> >>> Thanks. >>> >>>> logical, physical); >>>> } >>>> >>>> -- >>>> 2.44.0 >>>> >>>> >>>
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index 5e371ffdb37b..277583464371 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -860,10 +860,8 @@ static void scrub_stripe_submit_repair_read(struct scrub_stripe *stripe, static void scrub_stripe_report_errors(struct scrub_ctx *sctx, struct scrub_stripe *stripe) { - static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL, - DEFAULT_RATELIMIT_BURST); struct btrfs_fs_info *fs_info = sctx->fs_info; - struct btrfs_device *dev = NULL; + struct btrfs_device *dev = stripe->dev; u64 stripe_physical = stripe->physical; int nr_data_sectors = 0; int nr_meta_sectors = 0; @@ -874,35 +872,7 @@ static void scrub_stripe_report_errors(struct scrub_ctx *sctx, if (test_bit(SCRUB_STRIPE_FLAG_NO_REPORT, &stripe->state)) return; - /* - * Init needed infos for error reporting. - * - * Although our scrub_stripe infrastructure is mostly based on btrfs_submit_bio() - * thus no need for dev/physical, error reporting still needs dev and physical. - */ - if (!bitmap_empty(&stripe->init_error_bitmap, stripe->nr_sectors)) { - u64 mapped_len = fs_info->sectorsize; - struct btrfs_io_context *bioc = NULL; - int stripe_index = stripe->mirror_num - 1; - int ret; - - /* For scrub, our mirror_num should always start at 1. */ - ASSERT(stripe->mirror_num >= 1); - ret = btrfs_map_block(fs_info, BTRFS_MAP_GET_READ_MIRRORS, - stripe->logical, &mapped_len, &bioc, - NULL, NULL); - /* - * If we failed, dev will be NULL, and later detailed reports - * will just be skipped. - */ - if (ret < 0) - goto skip; - stripe_physical = bioc->stripes[stripe_index].physical; - dev = bioc->stripes[stripe_index].dev; - btrfs_put_bioc(bioc); - } - -skip: + ASSERT(dev); for_each_set_bit(sector_nr, &stripe->extent_sector_bitmap, stripe->nr_sectors) { u64 logical = stripe->logical + (sector_nr << fs_info->sectorsize_bits); @@ -933,42 +903,27 @@ static void scrub_stripe_report_errors(struct scrub_ctx *sctx, * output the message of repaired message. */ if (repaired) { - if (dev) { - btrfs_err_rl_in_rcu(fs_info, + btrfs_err_rl_in_rcu(fs_info, "fixed up error at logical %llu on dev %s physical %llu", logical, btrfs_dev_name(dev), physical); - } else { - btrfs_err_rl_in_rcu(fs_info, - "fixed up error at logical %llu on mirror %u", - logical, stripe->mirror_num); - } continue; } /* The remaining are all for unrepaired. */ - if (dev) { - btrfs_err_rl_in_rcu(fs_info, + btrfs_err_rl_in_rcu(fs_info, "unable to fixup (regular) error at logical %llu on dev %s physical %llu", logical, btrfs_dev_name(dev), physical); - } else { - btrfs_err_rl_in_rcu(fs_info, - "unable to fixup (regular) error at logical %llu on mirror %u", - logical, stripe->mirror_num); - } if (test_bit(sector_nr, &stripe->io_error_bitmap)) - if (__ratelimit(&rs) && dev) - scrub_print_common_warning("i/o error", dev, + scrub_print_common_warning("i/o error", dev, logical, physical); if (test_bit(sector_nr, &stripe->csum_error_bitmap)) - if (__ratelimit(&rs) && dev) - scrub_print_common_warning("checksum error", dev, + scrub_print_common_warning("checksum error", dev, logical, physical); if (test_bit(sector_nr, &stripe->meta_error_bitmap)) - if (__ratelimit(&rs) && dev) - scrub_print_common_warning("header error", dev, + scrub_print_common_warning("header error", dev, logical, physical); }
The @stripe passed into scrub_stripe_report_errors() either has stripe->dev and stripe->physical properly populated (regular data stripes) or stripe->flags would have SCRUB_STRIPE_FLAG_NO_REPORT (RAID56 P/Q stripes). Thus there is no need to go with btrfs_map_block() to get the dev/physical. Just add an extra ASSERT() to make sure we get stripe->dev populated correctly. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/scrub.c | 59 ++++++------------------------------------------ 1 file changed, 7 insertions(+), 52 deletions(-)