| Message ID | 20220510194338.24881-1-heinrich.schuchardt@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] btrfs: simplify lookup_data_extent() | expand |
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) > + 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) > + 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 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 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!
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 ||
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(-)