| Message ID | 20180726065334.30594-5-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Misc volume patch set part2 | expand |
On Thu, Jul 26, 2018 at 02:53:34PM +0800, Anand Jain wrote: > When the replace is running the fs_devices::num_devices also includes > the replace device, however in some operations like device delete and > balance it needs the actual num_devices without the repalce devices, so > now the function btrfs_num_devices() just provides that. The part how concurrent balance and dev-replace can be active at the same time is missing. As this is not obvious, it's desired to have that in the changelog. If there are questions and comments in past patchset revisions, that's a good material for changelogs and when you send an update you can enhance the changelogs. I do simple fixups or additions but when you send a new revision it the right time to save time on both sides. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/01/2018 10:41 PM, David Sterba wrote: > On Thu, Jul 26, 2018 at 02:53:34PM +0800, Anand Jain wrote: >> When the replace is running the fs_devices::num_devices also includes >> the replace device, however in some operations like device delete and >> balance it needs the actual num_devices without the repalce devices, so >> now the function btrfs_num_devices() just provides that. > > The part how concurrent balance and dev-replace can be active at the > same time is missing. As this is not obvious, it's desired to have that > in the changelog. Will do. > If there are questions and comments in past patchset revisions, that's a > good material for changelogs and when you send an update you can enhance > the changelogs. I do simple fixups or additions but when you send a new > revision it the right time to save time on both sides. Thanks for mentioning it. -Anand > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index fe74fefc75f7..8844904f9009 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1854,6 +1854,21 @@ void btrfs_assign_next_active_device(struct btrfs_device *device, fs_info->fs_devices->latest_bdev = next_device->bdev; } +/* Returns btrfs_fs_devices::num_devices excluding replace device if any */ +static inline u64 btrfs_num_devices(struct btrfs_fs_info *fs_info) +{ + u64 num_devices = fs_info->fs_devices->num_devices; + + btrfs_dev_replace_read_lock(&fs_info->dev_replace); + if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) { + BUG_ON(num_devices < 1); + num_devices--; + } + btrfs_dev_replace_read_unlock(&fs_info->dev_replace); + + return num_devices; +} + int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path, u64 devid) { @@ -1865,13 +1880,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path, mutex_lock(&uuid_mutex); - num_devices = fs_devices->num_devices; - btrfs_dev_replace_read_lock(&fs_info->dev_replace); - if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) { - BUG_ON(num_devices < 1); - num_devices--; - } - btrfs_dev_replace_read_unlock(&fs_info->dev_replace); + num_devices = btrfs_num_devices(fs_info); ret = btrfs_check_raid_min_devices(fs_info, num_devices - 1); if (ret) @@ -3721,13 +3730,8 @@ int btrfs_balance(struct btrfs_fs_info *fs_info, } } - num_devices = fs_info->fs_devices->num_devices; - btrfs_dev_replace_read_lock(&fs_info->dev_replace); - if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) { - BUG_ON(num_devices < 1); - num_devices--; - } - btrfs_dev_replace_read_unlock(&fs_info->dev_replace); + num_devices = btrfs_num_devices(fs_info); + allowed = BTRFS_AVAIL_ALLOC_BIT_SINGLE | BTRFS_BLOCK_GROUP_DUP; if (num_devices > 1) allowed |= (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1);
When the replace is running the fs_devices::num_devices also includes the replace device, however in some operations like device delete and balance it needs the actual num_devices without the repalce devices, so now the function btrfs_num_devices() just provides that. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- v1->v2: add comments fs/btrfs/volumes.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-)