[PATCH 1/1] btrfs: simplify lookup_data_extent()

After returning if ret <= 0 we know that ret > 0. No need to check it.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- fs/btrfs/inode.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index d00b515333..0173d30cd8 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -546,15 +546,12 @@ static int lookup_data_extent(struct btrfs_root *root, struct btrfs_path *path, /* Error or we're already at the file extent */ if (ret <= 0) return ret; - if (ret > 0) { - /* Check previous file extent */ - ret = btrfs_previous_item(root, path, ino, - BTRFS_EXTENT_DATA_KEY); - if (ret < 0) - return ret; - if (ret > 0) - goto check_next; - } + /* Check previous file extent */ + ret = btrfs_previous_item(root, path, ino, BTRFS_EXTENT_DATA_KEY); + if (ret < 0) + return ret; + if (ret > 0) + goto check_next; /* Now the key.offset must be smaller than @file_offset */ btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]); if (key.objectid != ino ||

On 2022/5/11 03:43, Heinrich Schuchardt wrote:
After returning if ret <= 0 we know that ret > 0. No need to check it.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Reviewed-by: Qu Wenruo wqu@suse.com
Thanks, Qu
fs/btrfs/inode.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index d00b515333..0173d30cd8 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -546,15 +546,12 @@ static int lookup_data_extent(struct btrfs_root *root, struct btrfs_path *path, /* Error or we're already at the file extent */ if (ret <= 0) return ret;
- if (ret > 0) {
/* Check previous file extent */
ret = btrfs_previous_item(root, path, ino,
BTRFS_EXTENT_DATA_KEY);
if (ret < 0)
return ret;
if (ret > 0)
goto check_next;
- }
- /* Check previous file extent */
- ret = btrfs_previous_item(root, path, ino, BTRFS_EXTENT_DATA_KEY);
- if (ret < 0)
return ret;
- if (ret > 0)
/* Now the key.offset must be smaller than @file_offset */ btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]); if (key.objectid != ino ||goto check_next;

On 2022/5/11 06:48, Qu Wenruo wrote:
On 2022/5/11 03:43, Heinrich Schuchardt wrote:
After returning if ret <= 0 we know that ret > 0. No need to check it.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Reviewed-by: Qu Wenruo wqu@suse.com
Just to mention for other guys in the btrfs list, this patch is for U-boot btrfs implementation.
And I also checked btrfs-fuse project, which has a similar function, lookup_file_extent(), it already does the check properly and even do extra quick exit for (ret > 0 && path->slots[0] == 0) case.
So you may want to also check btrfs-fuse project to find some possible optimization and cross-port to U-boot. (So far btrfs-fuse has better test coverage using fsstress, and cross-checked against kernel).
Thanks, Qu
Thanks, Qu
fs/btrfs/inode.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index d00b515333..0173d30cd8 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -546,15 +546,12 @@ static int lookup_data_extent(struct btrfs_root *root, struct btrfs_path *path, /* Error or we're already at the file extent */ if (ret <= 0) return ret; - if (ret > 0) { - /* Check previous file extent */ - ret = btrfs_previous_item(root, path, ino, - BTRFS_EXTENT_DATA_KEY); - if (ret < 0) - return ret; - if (ret > 0) - goto check_next; - } + /* Check previous file extent */ + ret = btrfs_previous_item(root, path, ino, BTRFS_EXTENT_DATA_KEY); + if (ret < 0) + return ret; + if (ret > 0) + goto check_next; /* Now the key.offset must be smaller than @file_offset */ btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]); if (key.objectid != ino ||

On 5/11/22 01:13, Heinrich Schuchardt wrote:
After returning if ret <= 0 we know that ret > 0. No need to check it.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Reviewed-by: Anand Jain <anand.jain>
fs/btrfs/inode.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index d00b515333..0173d30cd8 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -546,15 +546,12 @@ static int lookup_data_extent(struct btrfs_root *root, struct btrfs_path *path, /* Error or we're already at the file extent */ if (ret <= 0) return ret;
- if (ret > 0) {
/* Check previous file extent */
ret = btrfs_previous_item(root, path, ino,
BTRFS_EXTENT_DATA_KEY);
if (ret < 0)
return ret;
if (ret > 0)
goto check_next;
- }
- /* Check previous file extent */
- ret = btrfs_previous_item(root, path, ino, BTRFS_EXTENT_DATA_KEY);
- if (ret < 0)
return ret;
- if (ret > 0)
/* Now the key.offset must be smaller than @file_offset */ btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]); if (key.objectid != ino ||goto check_next;

On Tue, May 10, 2022 at 09:43:38PM +0200, Heinrich Schuchardt wrote:
After returning if ret <= 0 we know that ret > 0. No need to check it.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com Reviewed-by: Qu Wenruo wqu@suse.com Reviewed-by: Anand Jain <anand.jain>
Applied to u-boot/next, thanks!
participants (4)
-
Anand Jain
-
Heinrich Schuchardt
-
Qu Wenruo
-
Tom Rini