* Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function
2018-09-13 12:05 ` [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function Nikolay Borisov
@ 2018-09-13 23:05 ` Qu Wenruo
2018-10-31 9:35 ` misono.tomohiro
2018-10-31 9:40 ` misono.tomohiro
2 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2018-09-13 23:05 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
On 2018/9/13 下午8:05, Nikolay Borisov wrote:
> Since the inline extent code can be largely self-sufficient, factor
> it out from check_file_extent. No functional changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Indeed makes more sense to refactor inline check out.
Thanks,
Qu
> ---
> check/mode-lowmem.c | 142 ++++++++++++++++++++++++++------------------
> 1 file changed, 83 insertions(+), 59 deletions(-)
>
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1bce44f5658a..48c1537e7440 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
> return ret;
> }
>
> +
> +static int check_file_extent_inline(struct btrfs_root *root,
> + struct btrfs_path *path, u64 *size,
> + u64 *end)
> +{
> + u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> + BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> + struct extent_buffer *node = path->nodes[0];
> + struct btrfs_item *e = btrfs_item_nr(0);
> + struct btrfs_file_extent_item *fi;
> + struct btrfs_key fkey;
> + u64 extent_num_bytes;
> + u32 item_inline_len;
> + int ret;
> + int compressed = 0;
> + int err = 0;
> +
> + fi = btrfs_item_ptr(node, path->slots[0],
> + struct btrfs_file_extent_item);
> + item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> + extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> + compressed = btrfs_file_extent_compression(node, fi);
> + btrfs_item_key_to_cpu(node, &fkey, path->slots[0]);
> +
> + if (extent_num_bytes == 0) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> + root->objectid, fkey.objectid, fkey.offset);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + if (compressed) {
> + if (extent_num_bytes > root->fs_info->sectorsize) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
> + root->objectid, fkey.objectid,
> + fkey.offset, extent_num_bytes,
> + root->fs_info->sectorsize - 1);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + if (item_inline_len > max_inline_extent_size) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
> + root->objectid, fkey.objectid,
> + fkey.offset, item_inline_len,
> + max_inline_extent_size);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + } else {
> +
> + if (extent_num_bytes > max_inline_extent_size) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
> + root->objectid, fkey.objectid, fkey.offset,
> + extent_num_bytes, max_inline_extent_size);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + }
> + if (!compressed && extent_num_bytes != item_inline_len) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
> + root->objectid, fkey.objectid, fkey.offset,
> + extent_num_bytes, item_inline_len);
> + if (repair) {
> + ret = repair_inline_ram_bytes(root, path,
> + &extent_num_bytes);
> + if (ret)
> + err |= FILE_EXTENT_ERROR;
> + } else {
> + err |= FILE_EXTENT_ERROR;
> + }
> + }
> + *end += extent_num_bytes;
> + *size += extent_num_bytes;
> +
> + return err;
> +}
> +
> /*
> * Check file extent datasum/hole, update the size of the file extents,
> * check and update the last offset of the file extent.
> @@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
> u64 csum_found; /* In byte size, sectorsize aligned */
> u64 search_start; /* Logical range start we search for csum */
> u64 search_len; /* Logical range len we search for csum */
> - u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> - BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> unsigned int extent_type;
> unsigned int is_hole;
> int slot = path->slots[0];
> @@ -1838,63 +1917,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>
> /* Check inline extent */
> extent_type = btrfs_file_extent_type(node, fi);
> - if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> - struct btrfs_item *e = btrfs_item_nr(slot);
> - u32 item_inline_len;
> -
> - item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> - extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> - compressed = btrfs_file_extent_compression(node, fi);
> - if (extent_num_bytes == 0) {
> - error(
> - "root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> - root->objectid, fkey.objectid, fkey.offset);
> - err |= FILE_EXTENT_ERROR;
> - }
> - if (compressed) {
> - if (extent_num_bytes > root->fs_info->sectorsize) {
> - error(
> -"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
> - root->objectid, fkey.objectid,
> - fkey.offset, extent_num_bytes,
> - root->fs_info->sectorsize - 1);
> - err |= FILE_EXTENT_ERROR;
> - }
> - if (item_inline_len > max_inline_extent_size) {
> - error(
> -"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
> - root->objectid, fkey.objectid,
> - fkey.offset, item_inline_len,
> - max_inline_extent_size);
> - err |= FILE_EXTENT_ERROR;
> - }
> - } else {
> - if (extent_num_bytes > max_inline_extent_size) {
> - error(
> - "root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
> - root->objectid, fkey.objectid, fkey.offset,
> - extent_num_bytes, max_inline_extent_size);
> - err |= FILE_EXTENT_ERROR;
> - }
> - }
> - if (!compressed && extent_num_bytes != item_inline_len) {
> - error(
> - "root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
> - root->objectid, fkey.objectid, fkey.offset,
> - extent_num_bytes, item_inline_len);
> - if (repair) {
> - ret = repair_inline_ram_bytes(root, path,
> - &extent_num_bytes);
> - if (ret)
> - err |= FILE_EXTENT_ERROR;
> - } else {
> - err |= FILE_EXTENT_ERROR;
> - }
> - }
> - *end += extent_num_bytes;
> - *size += extent_num_bytes;
> - return err;
> - }
> + if (extent_type == BTRFS_FILE_EXTENT_INLINE)
> + return check_file_extent_inline(root, path, size, end);
>
> /* Check extent type */
> if (extent_type != BTRFS_FILE_EXTENT_REG &&
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function
2018-09-13 12:05 ` [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function Nikolay Borisov
2018-09-13 23:05 ` Qu Wenruo
@ 2018-10-31 9:35 ` misono.tomohiro
2018-10-31 9:43 ` Nikolay Borisov
2018-10-31 9:40 ` misono.tomohiro
2 siblings, 1 reply; 13+ messages in thread
From: misono.tomohiro @ 2018-10-31 9:35 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs@vger.kernel.org, David Sterba
Hello,
fsck-test 006 fails for low-mem mode in current devel branch and bisect points this.
> Since the inline extent code can be largely self-sufficient, factor
> it out from check_file_extent. No functional changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> check/mode-lowmem.c | 142 ++++++++++++++++++++++++++------------------
> 1 file changed, 83 insertions(+), 59 deletions(-)
>
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1bce44f5658a..48c1537e7440 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
> return ret;
> }
>
> +
> +static int check_file_extent_inline(struct btrfs_root *root,
> + struct btrfs_path *path, u64 *size,
> + u64 *end)
> +{
> + u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> + BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> + struct extent_buffer *node = path->nodes[0];
> + struct btrfs_item *e = btrfs_item_nr(0);
btrfs_item_nr(path->slots[0])
I think this fixes the problem.
Thanks.
Misono
> + struct btrfs_file_extent_item *fi;
> + struct btrfs_key fkey;
> + u64 extent_num_bytes;
> + u32 item_inline_len;
> + int ret;
> + int compressed = 0;
> + int err = 0;
> +
> + fi = btrfs_item_ptr(node, path->slots[0],
> + struct btrfs_file_extent_item);
> + item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> + extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> + compressed = btrfs_file_extent_compression(node, fi);
> + btrfs_item_key_to_cpu(node, &fkey, path->slots[0]);
> +
> + if (extent_num_bytes == 0) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> + root->objectid, fkey.objectid, fkey.offset);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + if (compressed) {
> + if (extent_num_bytes > root->fs_info->sectorsize) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
> + root->objectid, fkey.objectid,
> + fkey.offset, extent_num_bytes,
> + root->fs_info->sectorsize - 1);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + if (item_inline_len > max_inline_extent_size) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
> + root->objectid, fkey.objectid,
> + fkey.offset, item_inline_len,
> + max_inline_extent_size);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + } else {
> +
> + if (extent_num_bytes > max_inline_extent_size) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
> + root->objectid, fkey.objectid, fkey.offset,
> + extent_num_bytes, max_inline_extent_size);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + }
> + if (!compressed && extent_num_bytes != item_inline_len) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
> + root->objectid, fkey.objectid, fkey.offset,
> + extent_num_bytes, item_inline_len);
> + if (repair) {
> + ret = repair_inline_ram_bytes(root, path,
> + &extent_num_bytes);
> + if (ret)
> + err |= FILE_EXTENT_ERROR;
> + } else {
> + err |= FILE_EXTENT_ERROR;
> + }
> + }
> + *end += extent_num_bytes;
> + *size += extent_num_bytes;
> +
> + return err;
> +}
> +
> /*
> * Check file extent datasum/hole, update the size of the file extents,
> * check and update the last offset of the file extent.
> @@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
> u64 csum_found; /* In byte size, sectorsize aligned */
> u64 search_start; /* Logical range start we search for csum */
> u64 search_len; /* Logical range len we search for csum */
> - u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> - BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> unsigned int extent_type;
> unsigned int is_hole;
> int slot = path->slots[0];
> @@ -1838,63 +1917,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>
> /* Check inline extent */
> extent_type = btrfs_file_extent_type(node, fi);
> - if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> - struct btrfs_item *e = btrfs_item_nr(slot);
> - u32 item_inline_len;
> -
> - item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> - extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> - compressed = btrfs_file_extent_compression(node, fi);
> - if (extent_num_bytes == 0) {
> - error(
> - "root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> - root->objectid, fkey.objectid, fkey.offset);
> - err |= FILE_EXTENT_ERROR;
> - }
> - if (compressed) {
> - if (extent_num_bytes > root->fs_info->sectorsize) {
> - error(
> -"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
> - root->objectid, fkey.objectid,
> - fkey.offset, extent_num_bytes,
> - root->fs_info->sectorsize - 1);
> - err |= FILE_EXTENT_ERROR;
> - }
> - if (item_inline_len > max_inline_extent_size) {
> - error(
> -"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
> - root->objectid, fkey.objectid,
> - fkey.offset, item_inline_len,
> - max_inline_extent_size);
> - err |= FILE_EXTENT_ERROR;
> - }
> - } else {
> - if (extent_num_bytes > max_inline_extent_size) {
> - error(
> - "root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
> - root->objectid, fkey.objectid, fkey.offset,
> - extent_num_bytes, max_inline_extent_size);
> - err |= FILE_EXTENT_ERROR;
> - }
> - }
> - if (!compressed && extent_num_bytes != item_inline_len) {
> - error(
> - "root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
> - root->objectid, fkey.objectid, fkey.offset,
> - extent_num_bytes, item_inline_len);
> - if (repair) {
> - ret = repair_inline_ram_bytes(root, path,
> - &extent_num_bytes);
> - if (ret)
> - err |= FILE_EXTENT_ERROR;
> - } else {
> - err |= FILE_EXTENT_ERROR;
> - }
> - }
> - *end += extent_num_bytes;
> - *size += extent_num_bytes;
> - return err;
> - }
> + if (extent_type == BTRFS_FILE_EXTENT_INLINE)
> + return check_file_extent_inline(root, path, size, end);
>
> /* Check extent type */
> if (extent_type != BTRFS_FILE_EXTENT_REG &&
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function
2018-10-31 9:35 ` misono.tomohiro
@ 2018-10-31 9:43 ` Nikolay Borisov
2018-10-31 17:19 ` David Sterba
0 siblings, 1 reply; 13+ messages in thread
From: Nikolay Borisov @ 2018-10-31 9:43 UTC (permalink / raw)
To: misono.tomohiro@fujitsu.com, linux-btrfs@vger.kernel.org,
David Sterba
On 31.10.18 г. 11:35 ч., misono.tomohiro@fujitsu.com wrote:
> Hello,
>
> fsck-test 006 fails for low-mem mode in current devel branch and bisect points this.
>
>> Since the inline extent code can be largely self-sufficient, factor
>> it out from check_file_extent. No functional changes.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>> check/mode-lowmem.c | 142 ++++++++++++++++++++++++++------------------
>> 1 file changed, 83 insertions(+), 59 deletions(-)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 1bce44f5658a..48c1537e7440 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
>> return ret;
>> }
>>
>> +
>> +static int check_file_extent_inline(struct btrfs_root *root,
>> + struct btrfs_path *path, u64 *size,
>> + u64 *end)
>> +{
>> + u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
>> + BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>> + struct extent_buffer *node = path->nodes[0];
>> + struct btrfs_item *e = btrfs_item_nr(0);
> btrfs_item_nr(path->slots[0])
>
> I think this fixes the problem.
Indeed, the original code uses path->slots[0] as the slot whereas I've
fixed that at slow 0, which of course is not always going to be the case.
David will you fold this in the original patch ?
> Thanks.
>
> Misono
>
>> + struct btrfs_file_extent_item *fi;
>> + struct btrfs_key fkey;
>> + u64 extent_num_bytes;
>> + u32 item_inline_len;
>> + int ret;
>> + int compressed = 0;
>> + int err = 0;
>> +
>> + fi = btrfs_item_ptr(node, path->slots[0],
>> + struct btrfs_file_extent_item);
>> + item_inline_len = btrfs_file_extent_inline_item_len(node, e);
>> + extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
>> + compressed = btrfs_file_extent_compression(node, fi);
>> + btrfs_item_key_to_cpu(node, &fkey, path->slots[0]);
>> +
>> + if (extent_num_bytes == 0) {
>> + error(
>> +"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
>> + root->objectid, fkey.objectid, fkey.offset);
>> + err |= FILE_EXTENT_ERROR;
>> + }
>> +
>> + if (compressed) {
>> + if (extent_num_bytes > root->fs_info->sectorsize) {
>> + error(
>> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
>> + root->objectid, fkey.objectid,
>> + fkey.offset, extent_num_bytes,
>> + root->fs_info->sectorsize - 1);
>> + err |= FILE_EXTENT_ERROR;
>> + }
>> +
>> + if (item_inline_len > max_inline_extent_size) {
>> + error(
>> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
>> + root->objectid, fkey.objectid,
>> + fkey.offset, item_inline_len,
>> + max_inline_extent_size);
>> + err |= FILE_EXTENT_ERROR;
>> + }
>> +
>> + } else {
>> +
>> + if (extent_num_bytes > max_inline_extent_size) {
>> + error(
>> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
>> + root->objectid, fkey.objectid, fkey.offset,
>> + extent_num_bytes, max_inline_extent_size);
>> + err |= FILE_EXTENT_ERROR;
>> + }
>> +
>> + }
>> + if (!compressed && extent_num_bytes != item_inline_len) {
>> + error(
>> +"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
>> + root->objectid, fkey.objectid, fkey.offset,
>> + extent_num_bytes, item_inline_len);
>> + if (repair) {
>> + ret = repair_inline_ram_bytes(root, path,
>> + &extent_num_bytes);
>> + if (ret)
>> + err |= FILE_EXTENT_ERROR;
>> + } else {
>> + err |= FILE_EXTENT_ERROR;
>> + }
>> + }
>> + *end += extent_num_bytes;
>> + *size += extent_num_bytes;
>> +
>> + return err;
>> +}
>> +
>> /*
>> * Check file extent datasum/hole, update the size of the file extents,
>> * check and update the last offset of the file extent.
>> @@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>> u64 csum_found; /* In byte size, sectorsize aligned */
>> u64 search_start; /* Logical range start we search for csum */
>> u64 search_len; /* Logical range len we search for csum */
>> - u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
>> - BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>> unsigned int extent_type;
>> unsigned int is_hole;
>> int slot = path->slots[0];
>> @@ -1838,63 +1917,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>>
>> /* Check inline extent */
>> extent_type = btrfs_file_extent_type(node, fi);
>> - if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
>> - struct btrfs_item *e = btrfs_item_nr(slot);
>> - u32 item_inline_len;
>> -
>> - item_inline_len = btrfs_file_extent_inline_item_len(node, e);
>> - extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
>> - compressed = btrfs_file_extent_compression(node, fi);
>> - if (extent_num_bytes == 0) {
>> - error(
>> - "root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
>> - root->objectid, fkey.objectid, fkey.offset);
>> - err |= FILE_EXTENT_ERROR;
>> - }
>> - if (compressed) {
>> - if (extent_num_bytes > root->fs_info->sectorsize) {
>> - error(
>> -"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
>> - root->objectid, fkey.objectid,
>> - fkey.offset, extent_num_bytes,
>> - root->fs_info->sectorsize - 1);
>> - err |= FILE_EXTENT_ERROR;
>> - }
>> - if (item_inline_len > max_inline_extent_size) {
>> - error(
>> -"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
>> - root->objectid, fkey.objectid,
>> - fkey.offset, item_inline_len,
>> - max_inline_extent_size);
>> - err |= FILE_EXTENT_ERROR;
>> - }
>> - } else {
>> - if (extent_num_bytes > max_inline_extent_size) {
>> - error(
>> - "root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
>> - root->objectid, fkey.objectid, fkey.offset,
>> - extent_num_bytes, max_inline_extent_size);
>> - err |= FILE_EXTENT_ERROR;
>> - }
>> - }
>> - if (!compressed && extent_num_bytes != item_inline_len) {
>> - error(
>> - "root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
>> - root->objectid, fkey.objectid, fkey.offset,
>> - extent_num_bytes, item_inline_len);
>> - if (repair) {
>> - ret = repair_inline_ram_bytes(root, path,
>> - &extent_num_bytes);
>> - if (ret)
>> - err |= FILE_EXTENT_ERROR;
>> - } else {
>> - err |= FILE_EXTENT_ERROR;
>> - }
>> - }
>> - *end += extent_num_bytes;
>> - *size += extent_num_bytes;
>> - return err;
>> - }
>> + if (extent_type == BTRFS_FILE_EXTENT_INLINE)
>> + return check_file_extent_inline(root, path, size, end);
>>
>> /* Check extent type */
>> if (extent_type != BTRFS_FILE_EXTENT_REG &&
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function
2018-10-31 9:43 ` Nikolay Borisov
@ 2018-10-31 17:19 ` David Sterba
0 siblings, 0 replies; 13+ messages in thread
From: David Sterba @ 2018-10-31 17:19 UTC (permalink / raw)
To: Nikolay Borisov
Cc: misono.tomohiro@fujitsu.com, linux-btrfs@vger.kernel.org,
David Sterba
On Wed, Oct 31, 2018 at 11:43:20AM +0200, Nikolay Borisov wrote:
> >> --- a/check/mode-lowmem.c
> >> +++ b/check/mode-lowmem.c
> >> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
> >> return ret;
> >> }
> >>
> >> +
> >> +static int check_file_extent_inline(struct btrfs_root *root,
> >> + struct btrfs_path *path, u64 *size,
> >> + u64 *end)
> >> +{
> >> + u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> >> + BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> >> + struct extent_buffer *node = path->nodes[0];
> >> + struct btrfs_item *e = btrfs_item_nr(0);
> > btrfs_item_nr(path->slots[0])
> >
> > I think this fixes the problem.
>
> Indeed, the original code uses path->slots[0] as the slot whereas I've
> fixed that at slow 0, which of course is not always going to be the case.
>
> David will you fold this in the original patch ?
Yes I'll fold it. Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function
2018-09-13 12:05 ` [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function Nikolay Borisov
2018-09-13 23:05 ` Qu Wenruo
2018-10-31 9:35 ` misono.tomohiro
@ 2018-10-31 9:40 ` misono.tomohiro
2 siblings, 0 replies; 13+ messages in thread
From: misono.tomohiro @ 2018-10-31 9:40 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs@vger.kernel.org, David Sterba
Hello,
fsck-test 006 fails for low-mem mode in current devel branch and bisect points this.
On 2018/09/13 21:05, Nikolay Borisov wrote:
> Since the inline extent code can be largely self-sufficient, factor
> it out from check_file_extent. No functional changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> check/mode-lowmem.c | 142 ++++++++++++++++++++++++++------------------
> 1 file changed, 83 insertions(+), 59 deletions(-)
>
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1bce44f5658a..48c1537e7440 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
> return ret;
> }
>
> +
> +static int check_file_extent_inline(struct btrfs_root *root,
> + struct btrfs_path *path, u64 *size,
> + u64 *end)
> +{
> + u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> + BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> + struct extent_buffer *node = path->nodes[0];
> + struct btrfs_item *e = btrfs_item_nr(0);
btrfs_item_nr(path->slots[0])
I think this fixes the problem.
Thanks.
Misono
> + struct btrfs_file_extent_item *fi;
> + struct btrfs_key fkey;
> + u64 extent_num_bytes;
> + u32 item_inline_len;
> + int ret;
> + int compressed = 0;
> + int err = 0;
> +
> + fi = btrfs_item_ptr(node, path->slots[0],
> + struct btrfs_file_extent_item);
> + item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> + extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> + compressed = btrfs_file_extent_compression(node, fi);
> + btrfs_item_key_to_cpu(node, &fkey, path->slots[0]);
> +
> + if (extent_num_bytes == 0) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> + root->objectid, fkey.objectid, fkey.offset);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + if (compressed) {
> + if (extent_num_bytes > root->fs_info->sectorsize) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
> + root->objectid, fkey.objectid,
> + fkey.offset, extent_num_bytes,
> + root->fs_info->sectorsize - 1);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + if (item_inline_len > max_inline_extent_size) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
> + root->objectid, fkey.objectid,
> + fkey.offset, item_inline_len,
> + max_inline_extent_size);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + } else {
> +
> + if (extent_num_bytes > max_inline_extent_size) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
> + root->objectid, fkey.objectid, fkey.offset,
> + extent_num_bytes, max_inline_extent_size);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + }
> + if (!compressed && extent_num_bytes != item_inline_len) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
> + root->objectid, fkey.objectid, fkey.offset,
> + extent_num_bytes, item_inline_len);
> + if (repair) {
> + ret = repair_inline_ram_bytes(root, path,
> + &extent_num_bytes);
> + if (ret)
> + err |= FILE_EXTENT_ERROR;
> + } else {
> + err |= FILE_EXTENT_ERROR;
> + }
> + }
> + *end += extent_num_bytes;
> + *size += extent_num_bytes;
> +
> + return err;
> +}
> +
> /*
> * Check file extent datasum/hole, update the size of the file extents,
> * check and update the last offset of the file extent.
> @@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
> u64 csum_found; /* In byte size, sectorsize aligned */
> u64 search_start; /* Logical range start we search for csum */
> u64 search_len; /* Logical range len we search for csum */
> - u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> - BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> unsigned int extent_type;
> unsigned int is_hole;
> int slot = path->slots[0];
> @@ -1838,63 +1917,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>
> /* Check inline extent */
> extent_type = btrfs_file_extent_type(node, fi);
> - if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> - struct btrfs_item *e = btrfs_item_nr(slot);
> - u32 item_inline_len;
> -
> - item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> - extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> - compressed = btrfs_file_extent_compression(node, fi);
> - if (extent_num_bytes == 0) {
> - error(
> - "root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> - root->objectid, fkey.objectid, fkey.offset);
> - err |= FILE_EXTENT_ERROR;
> - }
> - if (compressed) {
> - if (extent_num_bytes > root->fs_info->sectorsize) {
> - error(
> -"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
> - root->objectid, fkey.objectid,
> - fkey.offset, extent_num_bytes,
> - root->fs_info->sectorsize - 1);
> - err |= FILE_EXTENT_ERROR;
> - }
> - if (item_inline_len > max_inline_extent_size) {
> - error(
> -"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
> - root->objectid, fkey.objectid,
> - fkey.offset, item_inline_len,
> - max_inline_extent_size);
> - err |= FILE_EXTENT_ERROR;
> - }
> - } else {
> - if (extent_num_bytes > max_inline_extent_size) {
> - error(
> - "root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
> - root->objectid, fkey.objectid, fkey.offset,
> - extent_num_bytes, max_inline_extent_size);
> - err |= FILE_EXTENT_ERROR;
> - }
> - }
> - if (!compressed && extent_num_bytes != item_inline_len) {
> - error(
> - "root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
> - root->objectid, fkey.objectid, fkey.offset,
> - extent_num_bytes, item_inline_len);
> - if (repair) {
> - ret = repair_inline_ram_bytes(root, path,
> - &extent_num_bytes);
> - if (ret)
> - err |= FILE_EXTENT_ERROR;
> - } else {
> - err |= FILE_EXTENT_ERROR;
> - }
> - }
> - *end += extent_num_bytes;
> - *size += extent_num_bytes;
> - return err;
> - }
> + if (extent_type == BTRFS_FILE_EXTENT_INLINE)
> + return check_file_extent_inline(root, path, size, end);
>
> /* Check extent type */
> if (extent_type != BTRFS_FILE_EXTENT_REG &&
>
^ permalink raw reply [flat|nested] 13+ messages in thread