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 <nborisov@xxxxxxxx>
Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>
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;
>