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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

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 ||