| Message ID | 9a09b2850b25de2eb9142d95bcdb1b46ff0207af.1686724789.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: scrub: remove unused btrfs_path in scrub_simple_mirror() | expand |
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, Jun 14, 2023 at 02:39:55PM +0800, Qu Wenruo wrote: > The @path in scrub_simple_mirror() is no longer utilized after commit > e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe > infrastructure"). > > Before that commit, we call find_first_extent_item() directly, which > needs a path and that path can be reused. > > But after that switch commit, the extent search is done inside > queue_scrub_stripe(), which will no longer accept a path from outside. > > So the @path variable can be removed safed. > > Fixes: e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe infrastructure") > Signed-off-by: Qu Wenruo <wqu@suse.com> Added to misc-next, thanks. > --- > fs/btrfs/scrub.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index 7bd446720104..be6efe9f3b55 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -1958,15 +1958,12 @@ static int scrub_simple_mirror(struct scrub_ctx *sctx, > struct btrfs_fs_info *fs_info = sctx->fs_info; > const u64 logical_end = logical_start + logical_length; > /* An artificial limit, inherit from old scrub behavior */ This comment became stale after e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe infrastructure") so I'll update the patch to remove it as well.
On 2023/6/15 00:33, David Sterba wrote: > On Wed, Jun 14, 2023 at 02:39:55PM +0800, Qu Wenruo wrote: >> The @path in scrub_simple_mirror() is no longer utilized after commit >> e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe >> infrastructure"). >> >> Before that commit, we call find_first_extent_item() directly, which >> needs a path and that path can be reused. >> >> But after that switch commit, the extent search is done inside >> queue_scrub_stripe(), which will no longer accept a path from outside. >> >> So the @path variable can be removed safed. >> >> Fixes: e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe infrastructure") >> Signed-off-by: Qu Wenruo <wqu@suse.com> > > Added to misc-next, thanks. BTW, I didn't see the patch merged in misc-next nor in the latest 6.5 pull request. Is it missing by somehow? Thanks, Qu > >> --- >> fs/btrfs/scrub.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c >> index 7bd446720104..be6efe9f3b55 100644 >> --- a/fs/btrfs/scrub.c >> +++ b/fs/btrfs/scrub.c >> @@ -1958,15 +1958,12 @@ static int scrub_simple_mirror(struct scrub_ctx *sctx, >> struct btrfs_fs_info *fs_info = sctx->fs_info; >> const u64 logical_end = logical_start + logical_length; >> /* An artificial limit, inherit from old scrub behavior */ > > This comment became stale after e02ee89baa66 ("btrfs: scrub: switch > scrub_simple_mirror() to scrub_stripe infrastructure") so I'll update > the patch to remove it as well.
On Tue, Jun 27, 2023 at 01:20:49PM +0800, Qu Wenruo wrote: > > > On 2023/6/15 00:33, David Sterba wrote: > > On Wed, Jun 14, 2023 at 02:39:55PM +0800, Qu Wenruo wrote: > >> The @path in scrub_simple_mirror() is no longer utilized after commit > >> e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe > >> infrastructure"). > >> > >> Before that commit, we call find_first_extent_item() directly, which > >> needs a path and that path can be reused. > >> > >> But after that switch commit, the extent search is done inside > >> queue_scrub_stripe(), which will no longer accept a path from outside. > >> > >> So the @path variable can be removed safed. > >> > >> Fixes: e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe infrastructure") > >> Signed-off-by: Qu Wenruo <wqu@suse.com> > > > > Added to misc-next, thanks. > > BTW, I didn't see the patch merged in misc-next nor in the latest 6.5 > pull request. > > Is it missing by somehow? Yes it got accidentaly lost, I don't know where. This could happen when I'm rebasing branches and pick the wrong reference commit. Now added back, thanks.
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index 7bd446720104..be6efe9f3b55 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -1958,15 +1958,12 @@ static int scrub_simple_mirror(struct scrub_ctx *sctx, struct btrfs_fs_info *fs_info = sctx->fs_info; const u64 logical_end = logical_start + logical_length; /* An artificial limit, inherit from old scrub behavior */ - struct btrfs_path path = { 0 }; u64 cur_logical = logical_start; int ret; /* The range must be inside the bg */ ASSERT(logical_start >= bg->start && logical_end <= bg->start + bg->length); - path.search_commit_root = 1; - path.skip_locking = 1; /* Go through each extent items inside the logical range */ while (cur_logical < logical_end) { u64 cur_physical = physical + cur_logical - logical_start; @@ -2010,7 +2007,6 @@ static int scrub_simple_mirror(struct scrub_ctx *sctx, /* Don't hold CPU for too long time */ cond_resched(); } - btrfs_release_path(&path); return ret; }
The @path in scrub_simple_mirror() is no longer utilized after commit e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe infrastructure"). Before that commit, we call find_first_extent_item() directly, which needs a path and that path can be reused. But after that switch commit, the extent search is done inside queue_scrub_stripe(), which will no longer accept a path from outside. So the @path variable can be removed safed. Fixes: e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe infrastructure") Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/scrub.c | 4 ---- 1 file changed, 4 deletions(-)