On 2017年09月23日 08:48, Liu Bo wrote:
On Sat, Sep 23, 2017 at 08:46:55AM +0800, Qu Wenruo wrote:


On 2017年09月23日 07:36, Liu Bo wrote:
This uses a bool 'do_backup' to help understand this piece of code.

Signed-off-by: Liu Bo <bo.li....@oracle.com>
---
This is based on a patch "Btrfs: do not backup tree roots when fsync".

   fs/btrfs/disk-io.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index cdb7043..9811b9d 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3691,6 +3691,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int 
max_mirrors)
        int max_errors;
        int total_errors = 0;
        u64 flags;
+       bool do_backup = (max_mirrors == 0);

Why not replacing @max_mirrors with @do_backup as parameter?

If I read the code correctly, max_mirrors is not just for deciding
backup.

That's strange.

write_all_supers() only uses @max_mirrors by passing it to write_dev_supers() and wait_dev_supers().

Both of the write/wait_dev_supers() will replace @max_mirrors to BTRFS_SUPER_MIRROR_MAX if it's zero.

Further more, all write_all_supers() callers just pass @max_mirrors as bool (either 0 or 1).

So I don't see any point not replacing the parameter as bool.

Thanks,
Qu

thanks,

-liubo


Thanks,
Qu
        do_barriers = !btrfs_test_opt(fs_info, NOBARRIER);
@@ -3699,7 +3700,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int 
max_mirrors)
         * not from fsync where the tree roots in fs_info have not
         * been consistent on disk.
         */
-       if (max_mirrors == 0)
+       if (do_backup)
                backup_super_roots(fs_info);
        sb = fs_info->super_for_commit;

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to