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

[Tom Rini]

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 
> Reviewed-by: Qu Wenruo 
> Reviewed-by: Anand Jain 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature

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

[Qu Wenruo]

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 


Reviewed-by: Qu Wenruo 


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], , path->slots[0]);
  if (key.objectid != ino ||

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

[Anand Jain]

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 


Reviewed-by: 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)
+   goto check_next;
/* Now the key.offset must be smaller than @file_offset */
btrfs_item_key_to_cpu(path->nodes[0], , path->slots[0]);
if (key.objectid != ino ||

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

[Qu Wenruo]

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 


Reviewed-by: Qu Wenruo 

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], , path->slots[0]);
if (key.objectid != ino ||