| Message ID | 2a831fc01d65612914702b968174945f2f7e1c79.1715783315.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | part3 trivial adjustments for return variable coding style | expand |
On Thu, May 16, 2024 at 07:12:10PM +0800, Anand Jain wrote: > Since err represents the function return value, rename it as ret, > and rename the original ret, which serves as a helper return value, > to found. Also, optimize the code to continue call btrfs_put_root() > for the rest of the root if even after btrfs_orphan_cleanup() returns > error. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > v3: Add a code comment. > v2: Rename to 'found' instead of 'ret2' (Josef). > Call btrfs_put_root() in the while-loop, avoids use of the variable > 'found' outside of the while loop (Qu). > Use 'unsigned int i' instead of 'int' (Goffredo). > > fs/btrfs/disk-io.c | 38 ++++++++++++++++++++------------------ > 1 file changed, 20 insertions(+), 18 deletions(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index a91a8056758a..d38cf973b02a 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -2925,22 +2925,23 @@ static int btrfs_cleanup_fs_roots(struct btrfs_fs_info *fs_info) > { > u64 root_objectid = 0; > struct btrfs_root *gang[8]; > - int i = 0; > - int err = 0; > - unsigned int ret = 0; > + int ret = 0; > > while (1) { > + unsigned int i; > + unsigned int found; > + > spin_lock(&fs_info->fs_roots_radix_lock); > - ret = radix_tree_gang_lookup(&fs_info->fs_roots_radix, > + found = radix_tree_gang_lookup(&fs_info->fs_roots_radix, > (void **)gang, root_objectid, > ARRAY_SIZE(gang)); > - if (!ret) { > + if (!found) { > spin_unlock(&fs_info->fs_roots_radix_lock); > break; > } > - root_objectid = btrfs_root_id(gang[ret - 1]) + 1; > + root_objectid = btrfs_root_id(gang[found - 1]) + 1; > > - for (i = 0; i < ret; i++) { > + for (i = 0; i < found; i++) { You could also move the declaration of 'i' to the for loop as you move the other definition anyway. > /* Avoid to grab roots in dead_roots. */ > if (btrfs_root_refs(&gang[i]->root_item) == 0) { > gang[i] = NULL; > @@ -2951,24 +2952,25 @@ static int btrfs_cleanup_fs_roots(struct btrfs_fs_info *fs_info) > } > spin_unlock(&fs_info->fs_roots_radix_lock); > > - for (i = 0; i < ret; i++) { > + for (i = 0; i < found; i++) { > if (!gang[i]) > continue; > root_objectid = btrfs_root_id(gang[i]); > - err = btrfs_orphan_cleanup(gang[i]); > - if (err) > - goto out; > + /* > + * Continue to release the remaining roots after the first > + * error without cleanup and preserve the first error > + * for the return. > + */ > + if (!ret) > + ret = btrfs_orphan_cleanup(gang[i]); > btrfs_put_root(gang[i]); > } > + if (ret) > + break; > + > root_objectid++; > } > -out: > - /* Release the uncleaned roots due to error. */ > - for (; i < ret; i++) { > - if (gang[i]) > - btrfs_put_root(gang[i]); > - } > - return err; > + return ret; > } > > /* > -- > 2.38.1 >
On 5/21/24 23:10, David Sterba wrote: > On Thu, May 16, 2024 at 07:12:10PM +0800, Anand Jain wrote: >> Since err represents the function return value, rename it as ret, >> and rename the original ret, which serves as a helper return value, >> to found. Also, optimize the code to continue call btrfs_put_root() >> for the rest of the root if even after btrfs_orphan_cleanup() returns >> error. >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> >> --- >> v3: Add a code comment. >> v2: Rename to 'found' instead of 'ret2' (Josef). >> Call btrfs_put_root() in the while-loop, avoids use of the variable >> 'found' outside of the while loop (Qu). >> Use 'unsigned int i' instead of 'int' (Goffredo). >> >> fs/btrfs/disk-io.c | 38 ++++++++++++++++++++------------------ >> 1 file changed, 20 insertions(+), 18 deletions(-) >> >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >> index a91a8056758a..d38cf973b02a 100644 >> --- a/fs/btrfs/disk-io.c >> +++ b/fs/btrfs/disk-io.c >> @@ -2925,22 +2925,23 @@ static int btrfs_cleanup_fs_roots(struct btrfs_fs_info *fs_info) >> { >> u64 root_objectid = 0; >> struct btrfs_root *gang[8]; >> - int i = 0; >> - int err = 0; >> - unsigned int ret = 0; >> + int ret = 0; >> >> while (1) { >> + unsigned int i; >> + unsigned int found; >> + >> spin_lock(&fs_info->fs_roots_radix_lock); >> - ret = radix_tree_gang_lookup(&fs_info->fs_roots_radix, >> + found = radix_tree_gang_lookup(&fs_info->fs_roots_radix, >> (void **)gang, root_objectid, >> ARRAY_SIZE(gang)); >> - if (!ret) { >> + if (!found) { >> spin_unlock(&fs_info->fs_roots_radix_lock); >> break; >> } >> - root_objectid = btrfs_root_id(gang[ret - 1]) + 1; >> + root_objectid = btrfs_root_id(gang[found - 1]) + 1; >> >> - for (i = 0; i < ret; i++) { >> + for (i = 0; i < found; i++) { > > You could also move the declaration of 'i' to the for loop as you move > the other definition anyway. Yep. Done in v4. Thanks. > >> /* Avoid to grab roots in dead_roots. */ >> if (btrfs_root_refs(&gang[i]->root_item) == 0) { >> gang[i] = NULL; >> @@ -2951,24 +2952,25 @@ static int btrfs_cleanup_fs_roots(struct btrfs_fs_info *fs_info) >> } >> spin_unlock(&fs_info->fs_roots_radix_lock); >> >> - for (i = 0; i < ret; i++) { >> + for (i = 0; i < found; i++) { >> if (!gang[i]) >> continue; >> root_objectid = btrfs_root_id(gang[i]); >> - err = btrfs_orphan_cleanup(gang[i]); >> - if (err) >> - goto out; >> + /* >> + * Continue to release the remaining roots after the first >> + * error without cleanup and preserve the first error >> + * for the return. >> + */ >> + if (!ret) >> + ret = btrfs_orphan_cleanup(gang[i]); >> btrfs_put_root(gang[i]); >> } >> + if (ret) >> + break; >> + >> root_objectid++; >> } >> -out: >> - /* Release the uncleaned roots due to error. */ >> - for (; i < ret; i++) { >> - if (gang[i]) >> - btrfs_put_root(gang[i]); >> - } >> - return err; >> + return ret; >> } >> >> /* >> -- >> 2.38.1 >>
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index a91a8056758a..d38cf973b02a 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -2925,22 +2925,23 @@ static int btrfs_cleanup_fs_roots(struct btrfs_fs_info *fs_info) { u64 root_objectid = 0; struct btrfs_root *gang[8]; - int i = 0; - int err = 0; - unsigned int ret = 0; + int ret = 0; while (1) { + unsigned int i; + unsigned int found; + spin_lock(&fs_info->fs_roots_radix_lock); - ret = radix_tree_gang_lookup(&fs_info->fs_roots_radix, + found = radix_tree_gang_lookup(&fs_info->fs_roots_radix, (void **)gang, root_objectid, ARRAY_SIZE(gang)); - if (!ret) { + if (!found) { spin_unlock(&fs_info->fs_roots_radix_lock); break; } - root_objectid = btrfs_root_id(gang[ret - 1]) + 1; + root_objectid = btrfs_root_id(gang[found - 1]) + 1; - for (i = 0; i < ret; i++) { + for (i = 0; i < found; i++) { /* Avoid to grab roots in dead_roots. */ if (btrfs_root_refs(&gang[i]->root_item) == 0) { gang[i] = NULL; @@ -2951,24 +2952,25 @@ static int btrfs_cleanup_fs_roots(struct btrfs_fs_info *fs_info) } spin_unlock(&fs_info->fs_roots_radix_lock); - for (i = 0; i < ret; i++) { + for (i = 0; i < found; i++) { if (!gang[i]) continue; root_objectid = btrfs_root_id(gang[i]); - err = btrfs_orphan_cleanup(gang[i]); - if (err) - goto out; + /* + * Continue to release the remaining roots after the first + * error without cleanup and preserve the first error + * for the return. + */ + if (!ret) + ret = btrfs_orphan_cleanup(gang[i]); btrfs_put_root(gang[i]); } + if (ret) + break; + root_objectid++; } -out: - /* Release the uncleaned roots due to error. */ - for (; i < ret; i++) { - if (gang[i]) - btrfs_put_root(gang[i]); - } - return err; + return ret; } /*
Since err represents the function return value, rename it as ret, and rename the original ret, which serves as a helper return value, to found. Also, optimize the code to continue call btrfs_put_root() for the rest of the root if even after btrfs_orphan_cleanup() returns error. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- v3: Add a code comment. v2: Rename to 'found' instead of 'ret2' (Josef). Call btrfs_put_root() in the while-loop, avoids use of the variable 'found' outside of the while loop (Qu). Use 'unsigned int i' instead of 'int' (Goffredo). fs/btrfs/disk-io.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-)