On 22.12.2017 00:42, Liu Bo wrote:
> This is a prepare work for the following extent map selftest, which
> runs tests against em merge logic.
> 
> Signed-off-by: Liu Bo <bo.li....@oracle.com>
> ---
>  fs/btrfs/ctree.h |   2 ++
>  fs/btrfs/inode.c | 101 
> ++++++++++++++++++++++++++++++-------------------------
>  2 files changed, 58 insertions(+), 45 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index b2e09fe..328f40f 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3148,6 +3148,8 @@ struct btrfs_delalloc_work 
> *btrfs_alloc_delalloc_work(struct inode *inode,
>                                                   int delay_iput);
>  void btrfs_wait_and_free_delalloc_work(struct btrfs_delalloc_work *work);
>  
> +int btrfs_add_extent_mapping(struct extent_map_tree *em_tree,
> +                          struct extent_map **em_in, u64 start, u64 len);
>  struct extent_map *btrfs_get_extent_fiemap(struct btrfs_inode *inode,
>               struct page *page, size_t pg_offset, u64 start,
>               u64 len, int create);
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index e1a7f3c..527df6f 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6911,6 +6911,61 @@ static noinline int uncompress_inline(struct 
> btrfs_path *path,
>       return ret;
>  }
>  
> +int btrfs_add_extent_mapping(struct extent_map_tree *em_tree,
> +                          struct extent_map **em_in, u64 start, u64 len)

How about adding the following comment above the function: 

/**                                                                             
 * btrfs_add_extent_mapping - try to add given extent mapping                   
 * @em_tree - the extent tree into which we want to add the mapping             
 * @em_in - extent we are inserting                                             
 * @start - the start of the logical range of the extent we are adding          
 * @len - logical length of the extent                                          
 *                                                                              
 * Insert @em_in into @em_tree. In case there is an overlapping range, handle   
 * the -EEXIST by either:                                                       
 * a) Returning the existing extent in @em_in if there is a full overlap        
 * b) Merge the extents if they are near each other.                            
 *                                                                              
 * Returns 0 on success or a negative error code                               
 *                                                                              
 */ 

Also one thing which I'm not very clear is why do we need the start/len, aren't 
those already set in em_in ?



> +{
> +     int ret;
> +     struct extent_map *em = *em_in;
> +
> +     ret = add_extent_mapping(em_tree, em, 0);
> +     /* it is possible that someone inserted the extent into the tree
> +      * while we had the lock dropped.  It is also possible that
> +      * an overlapping map exists in the tree
> +      */
> +     if (ret == -EEXIST) {
> +             struct extent_map *existing;
> +
> +             ret = 0;
> +
> +             existing = search_extent_mapping(em_tree, start, len);
> +
> +             /*
> +              * existing will always be non-NULL, since there must be
> +              * extent causing the -EEXIST.
> +              */
> +             if (existing->start == em->start &&
> +                 extent_map_end(existing) >= extent_map_end(em) &&
> +                 em->block_start == existing->block_start) {
> +                     /*
> +                      * The existing extent map already encompasses the
> +                      * entire extent map we tried to add.
> +                      */
> +                     free_extent_map(em);
> +                     *em_in = existing;
> +                     ret = 0;
> +             } else if (start >= extent_map_end(existing) ||
> +                 start <= existing->start) {
> +                     /*
> +                      * The existing extent map is the one nearest to
> +                      * the [start, start + len) range which overlaps
> +                      */
> +                     ret = merge_extent_mapping(em_tree, existing,
> +                                                em, start);
> +                     free_extent_map(existing);
> +                     if (ret) {
> +                             free_extent_map(em);
> +                             *em_in = NULL;
> +                     }
> +             } else {
> +                     free_extent_map(em);
> +                     *em_in = existing;
> +                     ret = 0;
> +             }
> +     }
> +     ASSERT(ret == 0 || ret == -EEXIST);
> +     return ret;
> +}
> +
>  /*
>   * a bit scary, this does extent mapping from logical file offset to the 
> disk.
>   * the ugly parts come from merging extents from the disk with the in-ram
> @@ -7147,51 +7202,7 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode 
> *inode,
>  
>       err = 0;
>       write_lock(&em_tree->lock);
> -     ret = add_extent_mapping(em_tree, em, 0);
> -     /* it is possible that someone inserted the extent into the tree
> -      * while we had the lock dropped.  It is also possible that
> -      * an overlapping map exists in the tree
> -      */
> -     if (ret == -EEXIST) {
> -             struct extent_map *existing;
> -
> -             ret = 0;
> -
> -             existing = search_extent_mapping(em_tree, start, len);
> -             /*
> -              * existing will always be non-NULL, since there must be
> -              * extent causing the -EEXIST.
> -              */
> -             if (existing->start == em->start &&
> -                 extent_map_end(existing) >= extent_map_end(em) &&
> -                 em->block_start == existing->block_start) {
> -                     /*
> -                      * The existing extent map already encompasses the
> -                      * entire extent map we tried to add.
> -                      */
> -                     free_extent_map(em);
> -                     em = existing;
> -                     err = 0;
> -
> -             } else if (start >= extent_map_end(existing) ||
> -                 start <= existing->start) {
> -                     /*
> -                      * The existing extent map is the one nearest to
> -                      * the [start, start + len) range which overlaps
> -                      */
> -                     err = merge_extent_mapping(em_tree, existing,
> -                                                em, start);
> -                     free_extent_map(existing);
> -                     if (err) {
> -                             free_extent_map(em);
> -                             em = NULL;
> -                     }
> -             } else {
> -                     free_extent_map(em);
> -                     em = existing;
> -                     err = 0;
> -             }
> -     }
> +     err = btrfs_add_extent_mapping(em_tree, &em, start, len);
>       write_unlock(&em_tree->lock);
>  out:
>  
> 
--
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