On 2018/12/17 下午4:35, Nikolay Borisov wrote:
> found_type really holds the type of extent and is guaranteed to to have
> a value between [0, 2]. The only time it can contain anything different
> is if btrfs_lookup_file_extent returned a positive value and the
> previous item is different than an extent. Avoid this situation by
> simply checking found_key.type rather than assigning the item type to
> found_type intermittently. Also make the variable an u8 to reduce stack
> usage. No functional changes.
> 
> Signed-off-by: Nikolay Borisov <[email protected]>

Reviewed-by: Qu Wenruo <[email protected]>

Indeed, abusing u64 for u8 makes it pretty confusing.

Now looks much better.

Thanks,
Qu

> ---
>  fs/btrfs/inode.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 73ae4428d0df..fe25f66a98d9 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6746,7 +6746,7 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode 
> *inode,
>       u64 extent_start = 0;
>       u64 extent_end = 0;
>       u64 objectid = btrfs_ino(inode);
> -     u32 found_type;
> +     u8 extent_type;
>       struct btrfs_path *path = NULL;
>       struct btrfs_root *root = inode->root;
>       struct btrfs_file_extent_item *item;
> @@ -6812,11 +6812,9 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode 
> *inode,
>       leaf = path->nodes[0];
>       item = btrfs_item_ptr(leaf, path->slots[0],
>                             struct btrfs_file_extent_item);
> -     /* are we inside the extent that was found? */
>       btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
> -     found_type = found_key.type;
>       if (found_key.objectid != objectid ||
> -         found_type != BTRFS_EXTENT_DATA_KEY) {
> +         found_key.type != BTRFS_EXTENT_DATA_KEY) {
>               /*
>                * If we backup past the first extent we want to move forward
>                * and see if there is an extent in front of us, otherwise we'll
> @@ -6827,16 +6825,16 @@ struct extent_map *btrfs_get_extent(struct 
> btrfs_inode *inode,
>               goto next;
>       }
>  
> -     found_type = btrfs_file_extent_type(leaf, item);
> +     extent_type = btrfs_file_extent_type(leaf, item);
>       extent_start = found_key.offset;
> -     if (found_type == BTRFS_FILE_EXTENT_REG ||
> -         found_type == BTRFS_FILE_EXTENT_PREALLOC) {
> +     if (extent_type == BTRFS_FILE_EXTENT_REG ||
> +         extent_type == BTRFS_FILE_EXTENT_PREALLOC) {
>               extent_end = extent_start +
>                      btrfs_file_extent_num_bytes(leaf, item);
>  
>               trace_btrfs_get_extent_show_fi_regular(inode, leaf, item,
>                                                      extent_start);
> -     } else if (found_type == BTRFS_FILE_EXTENT_INLINE) {
> +     } else if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
>               size_t size;
>  
>               size = btrfs_file_extent_ram_bytes(leaf, item);
> @@ -6877,10 +6875,10 @@ struct extent_map *btrfs_get_extent(struct 
> btrfs_inode *inode,
>       btrfs_extent_item_to_extent_map(inode, path, item,
>                       new_inline, em);
>  
> -     if (found_type == BTRFS_FILE_EXTENT_REG ||
> -         found_type == BTRFS_FILE_EXTENT_PREALLOC) {
> +     if (extent_type == BTRFS_FILE_EXTENT_REG ||
> +         extent_type == BTRFS_FILE_EXTENT_PREALLOC) {
>               goto insert;
> -     } else if (found_type == BTRFS_FILE_EXTENT_INLINE) {
> +     } else if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
>               unsigned long ptr;
>               char *map;
>               size_t size;
> 

Reply via email to