Hi, Anand Jain Thanks for review it.
> -----Original Message----- > From: Anand Jain [mailto:anand.j...@oracle.com] > Sent: Friday, July 17, 2015 5:12 PM > To: Zhaolei; linux-btrfs@vger.kernel.org > Subject: Re: [PATCH] btrfs: Add raid56 support for updating > num_tolerated_disk_barrier_failures in btrfs_balance() > > > > nice clean up thanks. but... more below. > > On 07/16/2015 08:15 PM, Zhaolei wrote: > > From: Zhao Lei <zhao...@cn.fujitsu.com> > > > > Code for updating fs_info->num_tolerated_disk_barrier_failures in > > btrfs_balance() lacks raid56 support. > > > > Reason: > > Above code was wroten in 2012-08-01, together with > > btrfs_calc_num_tolerated_disk_barrier_failures()'s first version. > > > > Then, btrfs_calc_num_tolerated_disk_barrier_failures() was updated > > later to support raid56, but code in btrfs_balance() was not > > updated together. > > > > Fix: > > Merge these similar code by adding a argument to > > btrfs_calc_num_tolerated_disk_barrier_failures() to make it > > support both case. > > > > It can fix this bug with a bonus of cleanup, and make these code > > never in current no-sync state from now on. > > > > Signed-off-by: Zhao Lei <zhao...@cn.fujitsu.com> > > --- > > fs/btrfs/disk-io.c | 9 +++++---- > > fs/btrfs/disk-io.h | 2 +- > > fs/btrfs/volumes.c | 28 +++++++++------------------- > > 3 files changed, 15 insertions(+), 24 deletions(-) > > > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index > > b6600c7..ac26111 100644 > > --- a/fs/btrfs/disk-io.c > > +++ b/fs/btrfs/disk-io.c > > @@ -2946,7 +2946,7 @@ retry_root_backup: > > goto fail_sysfs; > > } > > fs_info->num_tolerated_disk_barrier_failures = > > - btrfs_calc_num_tolerated_disk_barrier_failures(fs_info); > > + btrfs_calc_num_tolerated_disk_barrier_failures(fs_info, 0); > > if (fs_info->fs_devices->missing_devices > > > fs_info->num_tolerated_disk_barrier_failures && > > !(sb->s_flags & MS_RDONLY)) { > > @@ -3441,7 +3441,7 @@ static int barrier_all_devices(struct btrfs_fs_info > *info) > > } > > > > int btrfs_calc_num_tolerated_disk_barrier_failures( > > - struct btrfs_fs_info *fs_info) > > + struct btrfs_fs_info *fs_info, u64 extra_flags) > > { > > extra_flags not required. since .. more below. > > > struct btrfs_ioctl_space_info space; > > struct btrfs_space_info *sinfo; > > @@ -3481,7 +3481,7 @@ int > btrfs_calc_num_tolerated_disk_barrier_failures( > > &space); > > if (space.total_bytes == 0 || space.used_bytes == 0) > > continue; > > - flags = space.flags; > > + flags = space.flags | extra_flags; > > /* > > * return > > * 0: if dup, single or RAID0 is configured for @@ > > -3493,7 > > +3493,8 @@ int btrfs_calc_num_tolerated_disk_barrier_failures( > > */ > > if (num_tolerated_disk_barrier_failures > 0 && > > ((flags & (BTRFS_BLOCK_GROUP_DUP | > > - BTRFS_BLOCK_GROUP_RAID0)) || > > + BTRFS_BLOCK_GROUP_RAID0 | > > + BTRFS_AVAIL_ALLOC_BIT_SINGLE)) || > > ((flags & BTRFS_BLOCK_GROUP_PROFILE_MASK) == > 0))) > > num_tolerated_disk_barrier_failures = 0; > > else if (num_tolerated_disk_barrier_failures > 1 && diff > --git > > a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h index d4cbfee..aceaa8d > > 100644 > > --- a/fs/btrfs/disk-io.h > > +++ b/fs/btrfs/disk-io.h > > @@ -140,7 +140,7 @@ struct btrfs_root *btrfs_create_tree(struct > btrfs_trans_handle *trans, > > int btree_lock_page_hook(struct page *page, void *data, > > void (*flush_fn)(void *)); > > int btrfs_calc_num_tolerated_disk_barrier_failures( > > - struct btrfs_fs_info *fs_info); > > + struct btrfs_fs_info *fs_info, u64 extra_flags); > > int __init btrfs_end_io_wq_init(void); > > void btrfs_end_io_wq_exit(void); > > > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index > > fbe7c10..d739915 100644 > > --- a/fs/btrfs/volumes.c > > +++ b/fs/btrfs/volumes.c > > @@ -1812,7 +1812,8 @@ int btrfs_rm_device(struct btrfs_root *root, char > *device_path) > > } > > > > root->fs_info->num_tolerated_disk_barrier_failures = > > - btrfs_calc_num_tolerated_disk_barrier_failures(root->fs_info); > > + btrfs_calc_num_tolerated_disk_barrier_failures(root->fs_info, > > + 0); > > > > /* > > * at this point, the device is zero sized. We want to @@ -2342,7 > > +2343,8 @@ int btrfs_init_new_device(struct btrfs_root *root, char > *device_path) > > } > > > > root->fs_info->num_tolerated_disk_barrier_failures = > > - btrfs_calc_num_tolerated_disk_barrier_failures(root->fs_info); > > + btrfs_calc_num_tolerated_disk_barrier_failures(root->fs_info, > > + 0); > > ret = btrfs_commit_transaction(trans, root); > > > > if (seeding_dev) { > > @@ -3573,23 +3575,10 @@ int btrfs_balance(struct btrfs_balance_control > *bctl, > > } while (read_seqretry(&fs_info->profiles_lock, seq)); > > > > if (bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) { > > - int num_tolerated_disk_barrier_failures; > > - u64 target = bctl->sys.target; > > - > > - num_tolerated_disk_barrier_failures = > > - btrfs_calc_num_tolerated_disk_barrier_failures(fs_info); > > - if (num_tolerated_disk_barrier_failures > 0 && > > - (target & > > - (BTRFS_BLOCK_GROUP_DUP | BTRFS_BLOCK_GROUP_RAID0 | > > - BTRFS_AVAIL_ALLOC_BIT_SINGLE))) > > - num_tolerated_disk_barrier_failures = 0; > > - else if (num_tolerated_disk_barrier_failures > 1 && > > - (target & > > - (BTRFS_BLOCK_GROUP_RAID1 | > BTRFS_BLOCK_GROUP_RAID10))) > > - num_tolerated_disk_barrier_failures = 1; > > - > > fs_info->num_tolerated_disk_barrier_failures = > > - num_tolerated_disk_barrier_failures; > > + btrfs_calc_num_tolerated_disk_barrier_failures( > > + fs_info, > > + bctl->sys.target); > > } > > target is part of the user-end set item. please don't propagate > that to the function btrfs_calc_num_tolerated_disk_barrier_failures() > which is quite usefully used by many more functions. target must be > handled in here. > > Also, while you are here it looks like this and > btrfs_chunk_max_errors() can be merged as well. > Do you means use btrfs_chunk_max_errors() here to calculate s_info->num_tolerated_disk_barrier_failures here, instead of adding a extea argument to btrfs_calc_num_tolerated_disk_barrier_failures(), like: info->num_tolerated_disk_barrier_failures = min( btrfs_calc_num_tolerated_disk_barrier_failures(fs_info), btrfs_chunk_max_errors(bctl->sys.target) ); Thanks Zhaolei > Thanks. Anand > > > > ret = insert_balance_item(fs_info->tree_root, bctl); @@ -3616,7 > > +3605,8 @@ int btrfs_balance(struct btrfs_balance_control *bctl, > > > > if (bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) { > > fs_info->num_tolerated_disk_barrier_failures = > > - btrfs_calc_num_tolerated_disk_barrier_failures(fs_info); > > + btrfs_calc_num_tolerated_disk_barrier_failures(fs_info, > > + 0); > > } > > > > if (bargs) { > > -- 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
