| Message ID | 1489523641-10345-2-git-send-email-bo.li.liu@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
At 03/15/2017 04:33 AM, Liu Bo wrote: > We have similar code here and there, this merges them into a helper. > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> I really hate to delay the merge, but unfortunately it seems that read_one_chunk() still contains the open code. Sorry I didn't find it in previous version. Despite that, at least beased on v4.11-rc1 and inside volumes.c, it has no extra lookup_extent_mapping() can be cleaned up then. Thanks, Qu > --- > v2: add @length to the error message in get_chunk_map. > > fs/btrfs/extent_io.c | 3 +- > fs/btrfs/volumes.c | 163 +++++++++++++++++---------------------------------- > fs/btrfs/volumes.h | 2 +- > 3 files changed, 57 insertions(+), 111 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 28e8192..2eccabf 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -2003,14 +2003,13 @@ int repair_io_failure(struct btrfs_inode *inode, u64 start, u64 length, > u64 map_length = 0; > u64 sector; > struct btrfs_bio *bbio = NULL; > - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > int ret; > > ASSERT(!(fs_info->sb->s_flags & MS_RDONLY)); > BUG_ON(!mirror_num); > > /* we can't repair anything in raid56 yet */ > - if (btrfs_is_parity_mirror(map_tree, logical, length, mirror_num)) > + if (btrfs_is_parity_mirror(fs_info, logical, length, mirror_num)) > return 0; > > bio = btrfs_io_bio_alloc(GFP_NOFS, 1); > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 73d56ee..758a485 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -2795,10 +2795,38 @@ static int btrfs_del_sys_chunk(struct btrfs_fs_info *fs_info, > return ret; > } > > +static struct extent_map *get_chunk_map(struct btrfs_fs_info *fs_info, > + u64 logical, u64 length) > +{ > + struct extent_map_tree *em_tree; > + struct extent_map *em; > + > + em_tree = &fs_info->mapping_tree.map_tree; > + read_lock(&em_tree->lock); > + em = lookup_extent_mapping(em_tree, logical, length); > + read_unlock(&em_tree->lock); > + > + if (!em) { > + btrfs_crit(fs_info, "unable to find logical %llu length %llu", > + logical, length); > + return ERR_PTR(-EINVAL); > + } > + > + if (em->start > logical || em->start + em->len < logical) { > + btrfs_crit(fs_info, > + "found a bad mapping, wanted %llu-%llu, found %llu-%llu", > + logical, length, em->start, em->start + em->len); > + free_extent_map(em); > + return ERR_PTR(-EINVAL); > + } > + > + /* callers are responsible for dropping em's ref. */ > + return em; > +} > + > int btrfs_remove_chunk(struct btrfs_trans_handle *trans, > struct btrfs_fs_info *fs_info, u64 chunk_offset) > { > - struct extent_map_tree *em_tree; > struct extent_map *em; > struct map_lookup *map; > u64 dev_extent_len = 0; > @@ -2806,23 +2834,15 @@ int btrfs_remove_chunk(struct btrfs_trans_handle *trans, > int i, ret = 0; > struct btrfs_fs_devices *fs_devices = fs_info->fs_devices; > > - em_tree = &fs_info->mapping_tree.map_tree; > - > - read_lock(&em_tree->lock); > - em = lookup_extent_mapping(em_tree, chunk_offset, 1); > - read_unlock(&em_tree->lock); > - > - if (!em || em->start > chunk_offset || > - em->start + em->len < chunk_offset) { > + em = get_chunk_map(fs_info, chunk_offset, 1); > + if (IS_ERR(em)) { > /* > * This is a logic error, but we don't want to just rely on the > * user having built with ASSERT enabled, so if ASSERT doesn't > * do anything we still error out. > */ > ASSERT(0); > - if (em) > - free_extent_map(em); > - return -EINVAL; > + return PTR_ERR(em); > } > map = em->map_lookup; > mutex_lock(&fs_info->chunk_mutex); > @@ -4888,7 +4908,6 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans, > struct btrfs_device *device; > struct btrfs_chunk *chunk; > struct btrfs_stripe *stripe; > - struct extent_map_tree *em_tree; > struct extent_map *em; > struct map_lookup *map; > size_t item_size; > @@ -4897,24 +4916,9 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans, > int i = 0; > int ret = 0; > > - em_tree = &fs_info->mapping_tree.map_tree; > - read_lock(&em_tree->lock); > - em = lookup_extent_mapping(em_tree, chunk_offset, chunk_size); > - read_unlock(&em_tree->lock); > - > - if (!em) { > - btrfs_crit(fs_info, "unable to find logical %Lu len %Lu", > - chunk_offset, chunk_size); > - return -EINVAL; > - } > - > - if (em->start != chunk_offset || em->len != chunk_size) { > - btrfs_crit(fs_info, > - "found a bad mapping, wanted %Lu-%Lu, found %Lu-%Lu", > - chunk_offset, chunk_size, em->start, em->len); > - free_extent_map(em); > - return -EINVAL; > - } > + em = get_chunk_map(fs_info, chunk_offset, chunk_size); > + if (IS_ERR(em)) > + return PTR_ERR(em); > > map = em->map_lookup; > item_size = btrfs_chunk_item_size(map->num_stripes); > @@ -5055,15 +5059,12 @@ int btrfs_chunk_readonly(struct btrfs_fs_info *fs_info, u64 chunk_offset) > { > struct extent_map *em; > struct map_lookup *map; > - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > int readonly = 0; > int miss_ndevs = 0; > int i; > > - read_lock(&map_tree->map_tree.lock); > - em = lookup_extent_mapping(&map_tree->map_tree, chunk_offset, 1); > - read_unlock(&map_tree->map_tree.lock); > - if (!em) > + em = get_chunk_map(fs_info, chunk_offset, 1); > + if (IS_ERR(em)) > return 1; > > map = em->map_lookup; > @@ -5117,34 +5118,19 @@ void btrfs_mapping_tree_free(struct btrfs_mapping_tree *tree) > > int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len) > { > - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > struct extent_map *em; > struct map_lookup *map; > - struct extent_map_tree *em_tree = &map_tree->map_tree; > int ret; > > - read_lock(&em_tree->lock); > - em = lookup_extent_mapping(em_tree, logical, len); > - read_unlock(&em_tree->lock); > - > - /* > - * We could return errors for these cases, but that could get ugly and > - * we'd probably do the same thing which is just not do anything else > - * and exit, so return 1 so the callers don't try to use other copies. > - */ > - if (!em) { > - btrfs_crit(fs_info, "No mapping for %Lu-%Lu", logical, > - logical+len); > - return 1; > - } > - > - if (em->start > logical || em->start + em->len < logical) { > - btrfs_crit(fs_info, "Invalid mapping for %Lu-%Lu, got %Lu-%Lu", > - logical, logical+len, em->start, > - em->start + em->len); > - free_extent_map(em); > + em = get_chunk_map(fs_info, logical, len); > + if (IS_ERR(em)) > + /* > + * We could return errors for these cases, but that could get > + * ugly and we'd probably do the same thing which is just not do > + * anything else and exit, so return 1 so the callers don't try > + * to use other copies. > + */ > return 1; > - } > > map = em->map_lookup; > if (map->type & (BTRFS_BLOCK_GROUP_DUP | BTRFS_BLOCK_GROUP_RAID1)) > @@ -5173,15 +5159,11 @@ unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, > { > struct extent_map *em; > struct map_lookup *map; > - struct extent_map_tree *em_tree = &map_tree->map_tree; > unsigned long len = fs_info->sectorsize; > > - read_lock(&em_tree->lock); > - em = lookup_extent_mapping(em_tree, logical, len); > - read_unlock(&em_tree->lock); > - BUG_ON(!em); > + em = get_chunk_map(fs_info, logical, len); > + BUG_ON(IS_ERR(em)); > > - BUG_ON(em->start > logical || em->start + em->len < logical); > map = em->map_lookup; > if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) > len = map->stripe_len * nr_data_stripes(map); > @@ -5189,20 +5171,16 @@ unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, > return len; > } > > -int btrfs_is_parity_mirror(struct btrfs_mapping_tree *map_tree, > +int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, > u64 logical, u64 len, int mirror_num) > { > struct extent_map *em; > struct map_lookup *map; > - struct extent_map_tree *em_tree = &map_tree->map_tree; > int ret = 0; > > - read_lock(&em_tree->lock); > - em = lookup_extent_mapping(em_tree, logical, len); > - read_unlock(&em_tree->lock); > - BUG_ON(!em); > + em = get_chunk_map(fs_info, logical, len); > + BUG_ON(IS_ERR(em)); > > - BUG_ON(em->start > logical || em->start + em->len < logical); > map = em->map_lookup; > if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) > ret = 1; > @@ -5322,8 +5300,6 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, > { > struct extent_map *em; > struct map_lookup *map; > - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > - struct extent_map_tree *em_tree = &map_tree->map_tree; > u64 offset; > u64 stripe_offset; > u64 stripe_end_offset; > @@ -5345,23 +5321,9 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, > u64 physical_to_patch_in_first_stripe = 0; > u64 raid56_full_stripe_start = (u64)-1; > > - read_lock(&em_tree->lock); > - em = lookup_extent_mapping(em_tree, logical, *length); > - read_unlock(&em_tree->lock); > - > - if (!em) { > - btrfs_crit(fs_info, "unable to find logical %llu len %llu", > - logical, *length); > - return -EINVAL; > - } > - > - if (em->start > logical || em->start + em->len < logical) { > - btrfs_crit(fs_info, > - "found a bad mapping, wanted %Lu, found %Lu-%Lu", > - logical, em->start, em->start + em->len); > - free_extent_map(em); > - return -EINVAL; > - } > + em = get_chunk_map(fs_info, logical, *length); > + if (IS_ERR(em)) > + return PTR_ERR(em); > > map = em->map_lookup; > offset = logical - em->start; > @@ -5897,8 +5859,6 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, > u64 chunk_start, u64 physical, u64 devid, > u64 **logical, int *naddrs, int *stripe_len) > { > - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > - struct extent_map_tree *em_tree = &map_tree->map_tree; > struct extent_map *em; > struct map_lookup *map; > u64 *buf; > @@ -5908,24 +5868,11 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, > u64 rmap_len; > int i, j, nr = 0; > > - read_lock(&em_tree->lock); > - em = lookup_extent_mapping(em_tree, chunk_start, 1); > - read_unlock(&em_tree->lock); > - > - if (!em) { > - btrfs_err(fs_info, "couldn't find em for chunk %Lu", > - chunk_start); > + em = get_chunk_map(fs_info, chunk_start, 1); > + if (IS_ERR(em)) > return -EIO; > - } > > - if (em->start != chunk_start) { > - btrfs_err(fs_info, "bad chunk start, em=%Lu, wanted=%Lu", > - em->start, chunk_start); > - free_extent_map(em); > - return -EIO; > - } > map = em->map_lookup; > - > length = em->len; > rmap_len = map->stripe_len; > > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 59be812..6e93de0 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -475,7 +475,7 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_fs_info *fs_info, > void btrfs_init_dev_replace_tgtdev_for_resume(struct btrfs_fs_info *fs_info, > struct btrfs_device *tgtdev); > void btrfs_scratch_superblocks(struct block_device *bdev, const char *device_path); > -int btrfs_is_parity_mirror(struct btrfs_mapping_tree *map_tree, > +int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, > u64 logical, u64 len, int mirror_num); > unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, > struct btrfs_mapping_tree *map_tree, > -- 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 Wed, Mar 15, 2017 at 08:57:09AM +0800, Qu Wenruo wrote: > > > At 03/15/2017 04:33 AM, Liu Bo wrote: > > We have similar code here and there, this merges them into a helper. > > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > > I really hate to delay the merge, but unfortunately it seems that > read_one_chunk() still contains the open code. > > Sorry I didn't find it in previous version. > No worry, I left read_one_chunk as is by intention since it doesn't think searching failure as an error, it's called while reading chunk tree, so if the chunk has not mapped, then it gets to create it. Thanks, -liubo > Despite that, at least beased on v4.11-rc1 and inside volumes.c, it has no > extra lookup_extent_mapping() can be cleaned up then. > > Thanks, > Qu > > --- > > v2: add @length to the error message in get_chunk_map. > > > > fs/btrfs/extent_io.c | 3 +- > > fs/btrfs/volumes.c | 163 +++++++++++++++++---------------------------------- > > fs/btrfs/volumes.h | 2 +- > > 3 files changed, 57 insertions(+), 111 deletions(-) > > > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > > index 28e8192..2eccabf 100644 > > --- a/fs/btrfs/extent_io.c > > +++ b/fs/btrfs/extent_io.c > > @@ -2003,14 +2003,13 @@ int repair_io_failure(struct btrfs_inode *inode, u64 start, u64 length, > > u64 map_length = 0; > > u64 sector; > > struct btrfs_bio *bbio = NULL; > > - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > > int ret; > > > > ASSERT(!(fs_info->sb->s_flags & MS_RDONLY)); > > BUG_ON(!mirror_num); > > > > /* we can't repair anything in raid56 yet */ > > - if (btrfs_is_parity_mirror(map_tree, logical, length, mirror_num)) > > + if (btrfs_is_parity_mirror(fs_info, logical, length, mirror_num)) > > return 0; > > > > bio = btrfs_io_bio_alloc(GFP_NOFS, 1); > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > > index 73d56ee..758a485 100644 > > --- a/fs/btrfs/volumes.c > > +++ b/fs/btrfs/volumes.c > > @@ -2795,10 +2795,38 @@ static int btrfs_del_sys_chunk(struct btrfs_fs_info *fs_info, > > return ret; > > } > > > > +static struct extent_map *get_chunk_map(struct btrfs_fs_info *fs_info, > > + u64 logical, u64 length) > > +{ > > + struct extent_map_tree *em_tree; > > + struct extent_map *em; > > + > > + em_tree = &fs_info->mapping_tree.map_tree; > > + read_lock(&em_tree->lock); > > + em = lookup_extent_mapping(em_tree, logical, length); > > + read_unlock(&em_tree->lock); > > + > > + if (!em) { > > + btrfs_crit(fs_info, "unable to find logical %llu length %llu", > > + logical, length); > > + return ERR_PTR(-EINVAL); > > + } > > + > > + if (em->start > logical || em->start + em->len < logical) { > > + btrfs_crit(fs_info, > > + "found a bad mapping, wanted %llu-%llu, found %llu-%llu", > > + logical, length, em->start, em->start + em->len); > > + free_extent_map(em); > > + return ERR_PTR(-EINVAL); > > + } > > + > > + /* callers are responsible for dropping em's ref. */ > > + return em; > > +} > > + > > int btrfs_remove_chunk(struct btrfs_trans_handle *trans, > > struct btrfs_fs_info *fs_info, u64 chunk_offset) > > { > > - struct extent_map_tree *em_tree; > > struct extent_map *em; > > struct map_lookup *map; > > u64 dev_extent_len = 0; > > @@ -2806,23 +2834,15 @@ int btrfs_remove_chunk(struct btrfs_trans_handle *trans, > > int i, ret = 0; > > struct btrfs_fs_devices *fs_devices = fs_info->fs_devices; > > > > - em_tree = &fs_info->mapping_tree.map_tree; > > - > > - read_lock(&em_tree->lock); > > - em = lookup_extent_mapping(em_tree, chunk_offset, 1); > > - read_unlock(&em_tree->lock); > > - > > - if (!em || em->start > chunk_offset || > > - em->start + em->len < chunk_offset) { > > + em = get_chunk_map(fs_info, chunk_offset, 1); > > + if (IS_ERR(em)) { > > /* > > * This is a logic error, but we don't want to just rely on the > > * user having built with ASSERT enabled, so if ASSERT doesn't > > * do anything we still error out. > > */ > > ASSERT(0); > > - if (em) > > - free_extent_map(em); > > - return -EINVAL; > > + return PTR_ERR(em); > > } > > map = em->map_lookup; > > mutex_lock(&fs_info->chunk_mutex); > > @@ -4888,7 +4908,6 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans, > > struct btrfs_device *device; > > struct btrfs_chunk *chunk; > > struct btrfs_stripe *stripe; > > - struct extent_map_tree *em_tree; > > struct extent_map *em; > > struct map_lookup *map; > > size_t item_size; > > @@ -4897,24 +4916,9 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans, > > int i = 0; > > int ret = 0; > > > > - em_tree = &fs_info->mapping_tree.map_tree; > > - read_lock(&em_tree->lock); > > - em = lookup_extent_mapping(em_tree, chunk_offset, chunk_size); > > - read_unlock(&em_tree->lock); > > - > > - if (!em) { > > - btrfs_crit(fs_info, "unable to find logical %Lu len %Lu", > > - chunk_offset, chunk_size); > > - return -EINVAL; > > - } > > - > > - if (em->start != chunk_offset || em->len != chunk_size) { > > - btrfs_crit(fs_info, > > - "found a bad mapping, wanted %Lu-%Lu, found %Lu-%Lu", > > - chunk_offset, chunk_size, em->start, em->len); > > - free_extent_map(em); > > - return -EINVAL; > > - } > > + em = get_chunk_map(fs_info, chunk_offset, chunk_size); > > + if (IS_ERR(em)) > > + return PTR_ERR(em); > > > > map = em->map_lookup; > > item_size = btrfs_chunk_item_size(map->num_stripes); > > @@ -5055,15 +5059,12 @@ int btrfs_chunk_readonly(struct btrfs_fs_info *fs_info, u64 chunk_offset) > > { > > struct extent_map *em; > > struct map_lookup *map; > > - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > > int readonly = 0; > > int miss_ndevs = 0; > > int i; > > > > - read_lock(&map_tree->map_tree.lock); > > - em = lookup_extent_mapping(&map_tree->map_tree, chunk_offset, 1); > > - read_unlock(&map_tree->map_tree.lock); > > - if (!em) > > + em = get_chunk_map(fs_info, chunk_offset, 1); > > + if (IS_ERR(em)) > > return 1; > > > > map = em->map_lookup; > > @@ -5117,34 +5118,19 @@ void btrfs_mapping_tree_free(struct btrfs_mapping_tree *tree) > > > > int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len) > > { > > - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > > struct extent_map *em; > > struct map_lookup *map; > > - struct extent_map_tree *em_tree = &map_tree->map_tree; > > int ret; > > > > - read_lock(&em_tree->lock); > > - em = lookup_extent_mapping(em_tree, logical, len); > > - read_unlock(&em_tree->lock); > > - > > - /* > > - * We could return errors for these cases, but that could get ugly and > > - * we'd probably do the same thing which is just not do anything else > > - * and exit, so return 1 so the callers don't try to use other copies. > > - */ > > - if (!em) { > > - btrfs_crit(fs_info, "No mapping for %Lu-%Lu", logical, > > - logical+len); > > - return 1; > > - } > > - > > - if (em->start > logical || em->start + em->len < logical) { > > - btrfs_crit(fs_info, "Invalid mapping for %Lu-%Lu, got %Lu-%Lu", > > - logical, logical+len, em->start, > > - em->start + em->len); > > - free_extent_map(em); > > + em = get_chunk_map(fs_info, logical, len); > > + if (IS_ERR(em)) > > + /* > > + * We could return errors for these cases, but that could get > > + * ugly and we'd probably do the same thing which is just not do > > + * anything else and exit, so return 1 so the callers don't try > > + * to use other copies. > > + */ > > return 1; > > - } > > > > map = em->map_lookup; > > if (map->type & (BTRFS_BLOCK_GROUP_DUP | BTRFS_BLOCK_GROUP_RAID1)) > > @@ -5173,15 +5159,11 @@ unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, > > { > > struct extent_map *em; > > struct map_lookup *map; > > - struct extent_map_tree *em_tree = &map_tree->map_tree; > > unsigned long len = fs_info->sectorsize; > > > > - read_lock(&em_tree->lock); > > - em = lookup_extent_mapping(em_tree, logical, len); > > - read_unlock(&em_tree->lock); > > - BUG_ON(!em); > > + em = get_chunk_map(fs_info, logical, len); > > + BUG_ON(IS_ERR(em)); > > > > - BUG_ON(em->start > logical || em->start + em->len < logical); > > map = em->map_lookup; > > if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) > > len = map->stripe_len * nr_data_stripes(map); > > @@ -5189,20 +5171,16 @@ unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, > > return len; > > } > > > > -int btrfs_is_parity_mirror(struct btrfs_mapping_tree *map_tree, > > +int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, > > u64 logical, u64 len, int mirror_num) > > { > > struct extent_map *em; > > struct map_lookup *map; > > - struct extent_map_tree *em_tree = &map_tree->map_tree; > > int ret = 0; > > > > - read_lock(&em_tree->lock); > > - em = lookup_extent_mapping(em_tree, logical, len); > > - read_unlock(&em_tree->lock); > > - BUG_ON(!em); > > + em = get_chunk_map(fs_info, logical, len); > > + BUG_ON(IS_ERR(em)); > > > > - BUG_ON(em->start > logical || em->start + em->len < logical); > > map = em->map_lookup; > > if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) > > ret = 1; > > @@ -5322,8 +5300,6 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, > > { > > struct extent_map *em; > > struct map_lookup *map; > > - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > > - struct extent_map_tree *em_tree = &map_tree->map_tree; > > u64 offset; > > u64 stripe_offset; > > u64 stripe_end_offset; > > @@ -5345,23 +5321,9 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, > > u64 physical_to_patch_in_first_stripe = 0; > > u64 raid56_full_stripe_start = (u64)-1; > > > > - read_lock(&em_tree->lock); > > - em = lookup_extent_mapping(em_tree, logical, *length); > > - read_unlock(&em_tree->lock); > > - > > - if (!em) { > > - btrfs_crit(fs_info, "unable to find logical %llu len %llu", > > - logical, *length); > > - return -EINVAL; > > - } > > - > > - if (em->start > logical || em->start + em->len < logical) { > > - btrfs_crit(fs_info, > > - "found a bad mapping, wanted %Lu, found %Lu-%Lu", > > - logical, em->start, em->start + em->len); > > - free_extent_map(em); > > - return -EINVAL; > > - } > > + em = get_chunk_map(fs_info, logical, *length); > > + if (IS_ERR(em)) > > + return PTR_ERR(em); > > > > map = em->map_lookup; > > offset = logical - em->start; > > @@ -5897,8 +5859,6 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, > > u64 chunk_start, u64 physical, u64 devid, > > u64 **logical, int *naddrs, int *stripe_len) > > { > > - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > > - struct extent_map_tree *em_tree = &map_tree->map_tree; > > struct extent_map *em; > > struct map_lookup *map; > > u64 *buf; > > @@ -5908,24 +5868,11 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, > > u64 rmap_len; > > int i, j, nr = 0; > > > > - read_lock(&em_tree->lock); > > - em = lookup_extent_mapping(em_tree, chunk_start, 1); > > - read_unlock(&em_tree->lock); > > - > > - if (!em) { > > - btrfs_err(fs_info, "couldn't find em for chunk %Lu", > > - chunk_start); > > + em = get_chunk_map(fs_info, chunk_start, 1); > > + if (IS_ERR(em)) > > return -EIO; > > - } > > > > - if (em->start != chunk_start) { > > - btrfs_err(fs_info, "bad chunk start, em=%Lu, wanted=%Lu", > > - em->start, chunk_start); > > - free_extent_map(em); > > - return -EIO; > > - } > > map = em->map_lookup; > > - > > length = em->len; > > rmap_len = map->stripe_len; > > > > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > > index 59be812..6e93de0 100644 > > --- a/fs/btrfs/volumes.h > > +++ b/fs/btrfs/volumes.h > > @@ -475,7 +475,7 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_fs_info *fs_info, > > void btrfs_init_dev_replace_tgtdev_for_resume(struct btrfs_fs_info *fs_info, > > struct btrfs_device *tgtdev); > > void btrfs_scratch_superblocks(struct block_device *bdev, const char *device_path); > > -int btrfs_is_parity_mirror(struct btrfs_mapping_tree *map_tree, > > +int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, > > u64 logical, u64 len, int mirror_num); > > unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, > > struct btrfs_mapping_tree *map_tree, > > > > -- 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
At 03/15/2017 10:27 AM, Liu Bo wrote: > On Wed, Mar 15, 2017 at 08:57:09AM +0800, Qu Wenruo wrote: >> >> >> At 03/15/2017 04:33 AM, Liu Bo wrote: >>> We have similar code here and there, this merges them into a helper. >>> >>> Signed-off-by: Liu Bo <bo.li.liu@oracle.com> >> >> I really hate to delay the merge, but unfortunately it seems that >> read_one_chunk() still contains the open code. >> >> Sorry I didn't find it in previous version. >> > > No worry, I left read_one_chunk as is by intention since it doesn't > think searching failure as an error, it's called while reading chunk > tree, so if the chunk has not mapped, then it gets to create it. > > Thanks, > > -liubo You're right. Feel free to add my reviewed tag. Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com> Thanks, Qu > >> Despite that, at least beased on v4.11-rc1 and inside volumes.c, it has no >> extra lookup_extent_mapping() can be cleaned up then. >> >> Thanks, >> Qu >>> --- >>> v2: add @length to the error message in get_chunk_map. >>> >>> fs/btrfs/extent_io.c | 3 +- >>> fs/btrfs/volumes.c | 163 +++++++++++++++++---------------------------------- >>> fs/btrfs/volumes.h | 2 +- >>> 3 files changed, 57 insertions(+), 111 deletions(-) >>> >>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >>> index 28e8192..2eccabf 100644 >>> --- a/fs/btrfs/extent_io.c >>> +++ b/fs/btrfs/extent_io.c >>> @@ -2003,14 +2003,13 @@ int repair_io_failure(struct btrfs_inode *inode, u64 start, u64 length, >>> u64 map_length = 0; >>> u64 sector; >>> struct btrfs_bio *bbio = NULL; >>> - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; >>> int ret; >>> >>> ASSERT(!(fs_info->sb->s_flags & MS_RDONLY)); >>> BUG_ON(!mirror_num); >>> >>> /* we can't repair anything in raid56 yet */ >>> - if (btrfs_is_parity_mirror(map_tree, logical, length, mirror_num)) >>> + if (btrfs_is_parity_mirror(fs_info, logical, length, mirror_num)) >>> return 0; >>> >>> bio = btrfs_io_bio_alloc(GFP_NOFS, 1); >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >>> index 73d56ee..758a485 100644 >>> --- a/fs/btrfs/volumes.c >>> +++ b/fs/btrfs/volumes.c >>> @@ -2795,10 +2795,38 @@ static int btrfs_del_sys_chunk(struct btrfs_fs_info *fs_info, >>> return ret; >>> } >>> >>> +static struct extent_map *get_chunk_map(struct btrfs_fs_info *fs_info, >>> + u64 logical, u64 length) >>> +{ >>> + struct extent_map_tree *em_tree; >>> + struct extent_map *em; >>> + >>> + em_tree = &fs_info->mapping_tree.map_tree; >>> + read_lock(&em_tree->lock); >>> + em = lookup_extent_mapping(em_tree, logical, length); >>> + read_unlock(&em_tree->lock); >>> + >>> + if (!em) { >>> + btrfs_crit(fs_info, "unable to find logical %llu length %llu", >>> + logical, length); >>> + return ERR_PTR(-EINVAL); >>> + } >>> + >>> + if (em->start > logical || em->start + em->len < logical) { >>> + btrfs_crit(fs_info, >>> + "found a bad mapping, wanted %llu-%llu, found %llu-%llu", >>> + logical, length, em->start, em->start + em->len); >>> + free_extent_map(em); >>> + return ERR_PTR(-EINVAL); >>> + } >>> + >>> + /* callers are responsible for dropping em's ref. */ >>> + return em; >>> +} >>> + >>> int btrfs_remove_chunk(struct btrfs_trans_handle *trans, >>> struct btrfs_fs_info *fs_info, u64 chunk_offset) >>> { >>> - struct extent_map_tree *em_tree; >>> struct extent_map *em; >>> struct map_lookup *map; >>> u64 dev_extent_len = 0; >>> @@ -2806,23 +2834,15 @@ int btrfs_remove_chunk(struct btrfs_trans_handle *trans, >>> int i, ret = 0; >>> struct btrfs_fs_devices *fs_devices = fs_info->fs_devices; >>> >>> - em_tree = &fs_info->mapping_tree.map_tree; >>> - >>> - read_lock(&em_tree->lock); >>> - em = lookup_extent_mapping(em_tree, chunk_offset, 1); >>> - read_unlock(&em_tree->lock); >>> - >>> - if (!em || em->start > chunk_offset || >>> - em->start + em->len < chunk_offset) { >>> + em = get_chunk_map(fs_info, chunk_offset, 1); >>> + if (IS_ERR(em)) { >>> /* >>> * This is a logic error, but we don't want to just rely on the >>> * user having built with ASSERT enabled, so if ASSERT doesn't >>> * do anything we still error out. >>> */ >>> ASSERT(0); >>> - if (em) >>> - free_extent_map(em); >>> - return -EINVAL; >>> + return PTR_ERR(em); >>> } >>> map = em->map_lookup; >>> mutex_lock(&fs_info->chunk_mutex); >>> @@ -4888,7 +4908,6 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans, >>> struct btrfs_device *device; >>> struct btrfs_chunk *chunk; >>> struct btrfs_stripe *stripe; >>> - struct extent_map_tree *em_tree; >>> struct extent_map *em; >>> struct map_lookup *map; >>> size_t item_size; >>> @@ -4897,24 +4916,9 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans, >>> int i = 0; >>> int ret = 0; >>> >>> - em_tree = &fs_info->mapping_tree.map_tree; >>> - read_lock(&em_tree->lock); >>> - em = lookup_extent_mapping(em_tree, chunk_offset, chunk_size); >>> - read_unlock(&em_tree->lock); >>> - >>> - if (!em) { >>> - btrfs_crit(fs_info, "unable to find logical %Lu len %Lu", >>> - chunk_offset, chunk_size); >>> - return -EINVAL; >>> - } >>> - >>> - if (em->start != chunk_offset || em->len != chunk_size) { >>> - btrfs_crit(fs_info, >>> - "found a bad mapping, wanted %Lu-%Lu, found %Lu-%Lu", >>> - chunk_offset, chunk_size, em->start, em->len); >>> - free_extent_map(em); >>> - return -EINVAL; >>> - } >>> + em = get_chunk_map(fs_info, chunk_offset, chunk_size); >>> + if (IS_ERR(em)) >>> + return PTR_ERR(em); >>> >>> map = em->map_lookup; >>> item_size = btrfs_chunk_item_size(map->num_stripes); >>> @@ -5055,15 +5059,12 @@ int btrfs_chunk_readonly(struct btrfs_fs_info *fs_info, u64 chunk_offset) >>> { >>> struct extent_map *em; >>> struct map_lookup *map; >>> - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; >>> int readonly = 0; >>> int miss_ndevs = 0; >>> int i; >>> >>> - read_lock(&map_tree->map_tree.lock); >>> - em = lookup_extent_mapping(&map_tree->map_tree, chunk_offset, 1); >>> - read_unlock(&map_tree->map_tree.lock); >>> - if (!em) >>> + em = get_chunk_map(fs_info, chunk_offset, 1); >>> + if (IS_ERR(em)) >>> return 1; >>> >>> map = em->map_lookup; >>> @@ -5117,34 +5118,19 @@ void btrfs_mapping_tree_free(struct btrfs_mapping_tree *tree) >>> >>> int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len) >>> { >>> - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; >>> struct extent_map *em; >>> struct map_lookup *map; >>> - struct extent_map_tree *em_tree = &map_tree->map_tree; >>> int ret; >>> >>> - read_lock(&em_tree->lock); >>> - em = lookup_extent_mapping(em_tree, logical, len); >>> - read_unlock(&em_tree->lock); >>> - >>> - /* >>> - * We could return errors for these cases, but that could get ugly and >>> - * we'd probably do the same thing which is just not do anything else >>> - * and exit, so return 1 so the callers don't try to use other copies. >>> - */ >>> - if (!em) { >>> - btrfs_crit(fs_info, "No mapping for %Lu-%Lu", logical, >>> - logical+len); >>> - return 1; >>> - } >>> - >>> - if (em->start > logical || em->start + em->len < logical) { >>> - btrfs_crit(fs_info, "Invalid mapping for %Lu-%Lu, got %Lu-%Lu", >>> - logical, logical+len, em->start, >>> - em->start + em->len); >>> - free_extent_map(em); >>> + em = get_chunk_map(fs_info, logical, len); >>> + if (IS_ERR(em)) >>> + /* >>> + * We could return errors for these cases, but that could get >>> + * ugly and we'd probably do the same thing which is just not do >>> + * anything else and exit, so return 1 so the callers don't try >>> + * to use other copies. >>> + */ >>> return 1; >>> - } >>> >>> map = em->map_lookup; >>> if (map->type & (BTRFS_BLOCK_GROUP_DUP | BTRFS_BLOCK_GROUP_RAID1)) >>> @@ -5173,15 +5159,11 @@ unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, >>> { >>> struct extent_map *em; >>> struct map_lookup *map; >>> - struct extent_map_tree *em_tree = &map_tree->map_tree; >>> unsigned long len = fs_info->sectorsize; >>> >>> - read_lock(&em_tree->lock); >>> - em = lookup_extent_mapping(em_tree, logical, len); >>> - read_unlock(&em_tree->lock); >>> - BUG_ON(!em); >>> + em = get_chunk_map(fs_info, logical, len); >>> + BUG_ON(IS_ERR(em)); >>> >>> - BUG_ON(em->start > logical || em->start + em->len < logical); >>> map = em->map_lookup; >>> if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) >>> len = map->stripe_len * nr_data_stripes(map); >>> @@ -5189,20 +5171,16 @@ unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, >>> return len; >>> } >>> >>> -int btrfs_is_parity_mirror(struct btrfs_mapping_tree *map_tree, >>> +int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, >>> u64 logical, u64 len, int mirror_num) >>> { >>> struct extent_map *em; >>> struct map_lookup *map; >>> - struct extent_map_tree *em_tree = &map_tree->map_tree; >>> int ret = 0; >>> >>> - read_lock(&em_tree->lock); >>> - em = lookup_extent_mapping(em_tree, logical, len); >>> - read_unlock(&em_tree->lock); >>> - BUG_ON(!em); >>> + em = get_chunk_map(fs_info, logical, len); >>> + BUG_ON(IS_ERR(em)); >>> >>> - BUG_ON(em->start > logical || em->start + em->len < logical); >>> map = em->map_lookup; >>> if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) >>> ret = 1; >>> @@ -5322,8 +5300,6 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, >>> { >>> struct extent_map *em; >>> struct map_lookup *map; >>> - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; >>> - struct extent_map_tree *em_tree = &map_tree->map_tree; >>> u64 offset; >>> u64 stripe_offset; >>> u64 stripe_end_offset; >>> @@ -5345,23 +5321,9 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, >>> u64 physical_to_patch_in_first_stripe = 0; >>> u64 raid56_full_stripe_start = (u64)-1; >>> >>> - read_lock(&em_tree->lock); >>> - em = lookup_extent_mapping(em_tree, logical, *length); >>> - read_unlock(&em_tree->lock); >>> - >>> - if (!em) { >>> - btrfs_crit(fs_info, "unable to find logical %llu len %llu", >>> - logical, *length); >>> - return -EINVAL; >>> - } >>> - >>> - if (em->start > logical || em->start + em->len < logical) { >>> - btrfs_crit(fs_info, >>> - "found a bad mapping, wanted %Lu, found %Lu-%Lu", >>> - logical, em->start, em->start + em->len); >>> - free_extent_map(em); >>> - return -EINVAL; >>> - } >>> + em = get_chunk_map(fs_info, logical, *length); >>> + if (IS_ERR(em)) >>> + return PTR_ERR(em); >>> >>> map = em->map_lookup; >>> offset = logical - em->start; >>> @@ -5897,8 +5859,6 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, >>> u64 chunk_start, u64 physical, u64 devid, >>> u64 **logical, int *naddrs, int *stripe_len) >>> { >>> - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; >>> - struct extent_map_tree *em_tree = &map_tree->map_tree; >>> struct extent_map *em; >>> struct map_lookup *map; >>> u64 *buf; >>> @@ -5908,24 +5868,11 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, >>> u64 rmap_len; >>> int i, j, nr = 0; >>> >>> - read_lock(&em_tree->lock); >>> - em = lookup_extent_mapping(em_tree, chunk_start, 1); >>> - read_unlock(&em_tree->lock); >>> - >>> - if (!em) { >>> - btrfs_err(fs_info, "couldn't find em for chunk %Lu", >>> - chunk_start); >>> + em = get_chunk_map(fs_info, chunk_start, 1); >>> + if (IS_ERR(em)) >>> return -EIO; >>> - } >>> >>> - if (em->start != chunk_start) { >>> - btrfs_err(fs_info, "bad chunk start, em=%Lu, wanted=%Lu", >>> - em->start, chunk_start); >>> - free_extent_map(em); >>> - return -EIO; >>> - } >>> map = em->map_lookup; >>> - >>> length = em->len; >>> rmap_len = map->stripe_len; >>> >>> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h >>> index 59be812..6e93de0 100644 >>> --- a/fs/btrfs/volumes.h >>> +++ b/fs/btrfs/volumes.h >>> @@ -475,7 +475,7 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_fs_info *fs_info, >>> void btrfs_init_dev_replace_tgtdev_for_resume(struct btrfs_fs_info *fs_info, >>> struct btrfs_device *tgtdev); >>> void btrfs_scratch_superblocks(struct block_device *bdev, const char *device_path); >>> -int btrfs_is_parity_mirror(struct btrfs_mapping_tree *map_tree, >>> +int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, >>> u64 logical, u64 len, int mirror_num); >>> unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, >>> struct btrfs_mapping_tree *map_tree, >>> >> >> > > -- 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/extent_io.c b/fs/btrfs/extent_io.c index 28e8192..2eccabf 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -2003,14 +2003,13 @@ int repair_io_failure(struct btrfs_inode *inode, u64 start, u64 length, u64 map_length = 0; u64 sector; struct btrfs_bio *bbio = NULL; - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; int ret; ASSERT(!(fs_info->sb->s_flags & MS_RDONLY)); BUG_ON(!mirror_num); /* we can't repair anything in raid56 yet */ - if (btrfs_is_parity_mirror(map_tree, logical, length, mirror_num)) + if (btrfs_is_parity_mirror(fs_info, logical, length, mirror_num)) return 0; bio = btrfs_io_bio_alloc(GFP_NOFS, 1); diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 73d56ee..758a485 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2795,10 +2795,38 @@ static int btrfs_del_sys_chunk(struct btrfs_fs_info *fs_info, return ret; } +static struct extent_map *get_chunk_map(struct btrfs_fs_info *fs_info, + u64 logical, u64 length) +{ + struct extent_map_tree *em_tree; + struct extent_map *em; + + em_tree = &fs_info->mapping_tree.map_tree; + read_lock(&em_tree->lock); + em = lookup_extent_mapping(em_tree, logical, length); + read_unlock(&em_tree->lock); + + if (!em) { + btrfs_crit(fs_info, "unable to find logical %llu length %llu", + logical, length); + return ERR_PTR(-EINVAL); + } + + if (em->start > logical || em->start + em->len < logical) { + btrfs_crit(fs_info, + "found a bad mapping, wanted %llu-%llu, found %llu-%llu", + logical, length, em->start, em->start + em->len); + free_extent_map(em); + return ERR_PTR(-EINVAL); + } + + /* callers are responsible for dropping em's ref. */ + return em; +} + int btrfs_remove_chunk(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, u64 chunk_offset) { - struct extent_map_tree *em_tree; struct extent_map *em; struct map_lookup *map; u64 dev_extent_len = 0; @@ -2806,23 +2834,15 @@ int btrfs_remove_chunk(struct btrfs_trans_handle *trans, int i, ret = 0; struct btrfs_fs_devices *fs_devices = fs_info->fs_devices; - em_tree = &fs_info->mapping_tree.map_tree; - - read_lock(&em_tree->lock); - em = lookup_extent_mapping(em_tree, chunk_offset, 1); - read_unlock(&em_tree->lock); - - if (!em || em->start > chunk_offset || - em->start + em->len < chunk_offset) { + em = get_chunk_map(fs_info, chunk_offset, 1); + if (IS_ERR(em)) { /* * This is a logic error, but we don't want to just rely on the * user having built with ASSERT enabled, so if ASSERT doesn't * do anything we still error out. */ ASSERT(0); - if (em) - free_extent_map(em); - return -EINVAL; + return PTR_ERR(em); } map = em->map_lookup; mutex_lock(&fs_info->chunk_mutex); @@ -4888,7 +4908,6 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans, struct btrfs_device *device; struct btrfs_chunk *chunk; struct btrfs_stripe *stripe; - struct extent_map_tree *em_tree; struct extent_map *em; struct map_lookup *map; size_t item_size; @@ -4897,24 +4916,9 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans, int i = 0; int ret = 0; - em_tree = &fs_info->mapping_tree.map_tree; - read_lock(&em_tree->lock); - em = lookup_extent_mapping(em_tree, chunk_offset, chunk_size); - read_unlock(&em_tree->lock); - - if (!em) { - btrfs_crit(fs_info, "unable to find logical %Lu len %Lu", - chunk_offset, chunk_size); - return -EINVAL; - } - - if (em->start != chunk_offset || em->len != chunk_size) { - btrfs_crit(fs_info, - "found a bad mapping, wanted %Lu-%Lu, found %Lu-%Lu", - chunk_offset, chunk_size, em->start, em->len); - free_extent_map(em); - return -EINVAL; - } + em = get_chunk_map(fs_info, chunk_offset, chunk_size); + if (IS_ERR(em)) + return PTR_ERR(em); map = em->map_lookup; item_size = btrfs_chunk_item_size(map->num_stripes); @@ -5055,15 +5059,12 @@ int btrfs_chunk_readonly(struct btrfs_fs_info *fs_info, u64 chunk_offset) { struct extent_map *em; struct map_lookup *map; - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; int readonly = 0; int miss_ndevs = 0; int i; - read_lock(&map_tree->map_tree.lock); - em = lookup_extent_mapping(&map_tree->map_tree, chunk_offset, 1); - read_unlock(&map_tree->map_tree.lock); - if (!em) + em = get_chunk_map(fs_info, chunk_offset, 1); + if (IS_ERR(em)) return 1; map = em->map_lookup; @@ -5117,34 +5118,19 @@ void btrfs_mapping_tree_free(struct btrfs_mapping_tree *tree) int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len) { - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; struct extent_map *em; struct map_lookup *map; - struct extent_map_tree *em_tree = &map_tree->map_tree; int ret; - read_lock(&em_tree->lock); - em = lookup_extent_mapping(em_tree, logical, len); - read_unlock(&em_tree->lock); - - /* - * We could return errors for these cases, but that could get ugly and - * we'd probably do the same thing which is just not do anything else - * and exit, so return 1 so the callers don't try to use other copies. - */ - if (!em) { - btrfs_crit(fs_info, "No mapping for %Lu-%Lu", logical, - logical+len); - return 1; - } - - if (em->start > logical || em->start + em->len < logical) { - btrfs_crit(fs_info, "Invalid mapping for %Lu-%Lu, got %Lu-%Lu", - logical, logical+len, em->start, - em->start + em->len); - free_extent_map(em); + em = get_chunk_map(fs_info, logical, len); + if (IS_ERR(em)) + /* + * We could return errors for these cases, but that could get + * ugly and we'd probably do the same thing which is just not do + * anything else and exit, so return 1 so the callers don't try + * to use other copies. + */ return 1; - } map = em->map_lookup; if (map->type & (BTRFS_BLOCK_GROUP_DUP | BTRFS_BLOCK_GROUP_RAID1)) @@ -5173,15 +5159,11 @@ unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, { struct extent_map *em; struct map_lookup *map; - struct extent_map_tree *em_tree = &map_tree->map_tree; unsigned long len = fs_info->sectorsize; - read_lock(&em_tree->lock); - em = lookup_extent_mapping(em_tree, logical, len); - read_unlock(&em_tree->lock); - BUG_ON(!em); + em = get_chunk_map(fs_info, logical, len); + BUG_ON(IS_ERR(em)); - BUG_ON(em->start > logical || em->start + em->len < logical); map = em->map_lookup; if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) len = map->stripe_len * nr_data_stripes(map); @@ -5189,20 +5171,16 @@ unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, return len; } -int btrfs_is_parity_mirror(struct btrfs_mapping_tree *map_tree, +int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, u64 logical, u64 len, int mirror_num) { struct extent_map *em; struct map_lookup *map; - struct extent_map_tree *em_tree = &map_tree->map_tree; int ret = 0; - read_lock(&em_tree->lock); - em = lookup_extent_mapping(em_tree, logical, len); - read_unlock(&em_tree->lock); - BUG_ON(!em); + em = get_chunk_map(fs_info, logical, len); + BUG_ON(IS_ERR(em)); - BUG_ON(em->start > logical || em->start + em->len < logical); map = em->map_lookup; if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) ret = 1; @@ -5322,8 +5300,6 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, { struct extent_map *em; struct map_lookup *map; - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; - struct extent_map_tree *em_tree = &map_tree->map_tree; u64 offset; u64 stripe_offset; u64 stripe_end_offset; @@ -5345,23 +5321,9 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, u64 physical_to_patch_in_first_stripe = 0; u64 raid56_full_stripe_start = (u64)-1; - read_lock(&em_tree->lock); - em = lookup_extent_mapping(em_tree, logical, *length); - read_unlock(&em_tree->lock); - - if (!em) { - btrfs_crit(fs_info, "unable to find logical %llu len %llu", - logical, *length); - return -EINVAL; - } - - if (em->start > logical || em->start + em->len < logical) { - btrfs_crit(fs_info, - "found a bad mapping, wanted %Lu, found %Lu-%Lu", - logical, em->start, em->start + em->len); - free_extent_map(em); - return -EINVAL; - } + em = get_chunk_map(fs_info, logical, *length); + if (IS_ERR(em)) + return PTR_ERR(em); map = em->map_lookup; offset = logical - em->start; @@ -5897,8 +5859,6 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, u64 chunk_start, u64 physical, u64 devid, u64 **logical, int *naddrs, int *stripe_len) { - struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; - struct extent_map_tree *em_tree = &map_tree->map_tree; struct extent_map *em; struct map_lookup *map; u64 *buf; @@ -5908,24 +5868,11 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, u64 rmap_len; int i, j, nr = 0; - read_lock(&em_tree->lock); - em = lookup_extent_mapping(em_tree, chunk_start, 1); - read_unlock(&em_tree->lock); - - if (!em) { - btrfs_err(fs_info, "couldn't find em for chunk %Lu", - chunk_start); + em = get_chunk_map(fs_info, chunk_start, 1); + if (IS_ERR(em)) return -EIO; - } - if (em->start != chunk_start) { - btrfs_err(fs_info, "bad chunk start, em=%Lu, wanted=%Lu", - em->start, chunk_start); - free_extent_map(em); - return -EIO; - } map = em->map_lookup; - length = em->len; rmap_len = map->stripe_len; diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 59be812..6e93de0 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -475,7 +475,7 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_fs_info *fs_info, void btrfs_init_dev_replace_tgtdev_for_resume(struct btrfs_fs_info *fs_info, struct btrfs_device *tgtdev); void btrfs_scratch_superblocks(struct block_device *bdev, const char *device_path); -int btrfs_is_parity_mirror(struct btrfs_mapping_tree *map_tree, +int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, u64 logical, u64 len, int mirror_num); unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info, struct btrfs_mapping_tree *map_tree,
We have similar code here and there, this merges them into a helper. Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- v2: add @length to the error message in get_chunk_map. fs/btrfs/extent_io.c | 3 +- fs/btrfs/volumes.c | 163 +++++++++++++++++---------------------------------- fs/btrfs/volumes.h | 2 +- 3 files changed, 57 insertions(+), 111 deletions(-)