On Mon, Dec 17, 2018 at 10:36:01AM +0200, Nikolay Borisov wrote:
> The code which executes if path->slots[0] points to an item not
> belonging to the inode we are interested in or is not na extent at all
> could really trigger if btrfs_lookup_file_extent returned > 0 and
> subsequently path->slots[0] was decremented. Make this explicit by
> moving the respective code in the correct if branch. No functional
> changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
> fs/btrfs/inode.c | 36 ++++++++++++++++++++----------------
> 1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 511d3b314af2..e8284cd0a122 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6805,22 +6805,26 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
> if (path->slots[0] == 0)
> goto not_found;
> path->slots[0]--;
> - }
> -
> - leaf = path->nodes[0];
> - item = btrfs_item_ptr(leaf, path->slots[0],
> - struct btrfs_file_extent_item);
> - btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
> - if (found_key.objectid != objectid ||
> - 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
> - * say there is a hole for our whole search range which can
> - * cause problems.
> - */
> - extent_end = start;
> - goto next;
> + leaf = path->nodes[0];
> + item = btrfs_item_ptr(leaf, path->slots[0],
> + struct btrfs_file_extent_item);
> + btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
> + if (found_key.objectid != objectid ||
> + 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 say there is a hole for our whole
> + * search range which can cause problems.
> + */
> + extent_end = start;
> + goto next;
> + }
> + } else {
> + leaf = path->nodes[0];
> + item = btrfs_item_ptr(leaf, path->slots[0],
> + struct btrfs_file_extent_item);
This is repeating code and could be simplified ... to the original code.
I'm not sure this patch is an improvement.