Re: [PATCH v3 4/6] btrfs-progs: lowmem check: Add dev_item check for used bytes and total bytes
On 10/09/2018 03:14 AM, Qu Wenruo wrote:
>
>
> On 2018/10/9 上午6:20, Hans van Kranenburg wrote:
>> On 10/08/2018 02:30 PM, Qu Wenruo wrote:
>>> Obviously, used bytes can't be larger than total bytes.
>>>
>>> Signed-off-by: Qu Wenruo
>>> ---
>>> check/mode-lowmem.c | 5 +
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>>> index 07c03cad77af..1173b963b8f3 100644
>>> --- a/check/mode-lowmem.c
>>> +++ b/check/mode-lowmem.c
>>> @@ -4074,6 +4074,11 @@ static int check_dev_item(struct btrfs_fs_info
>>> *fs_info,
>>> used = btrfs_device_bytes_used(eb, dev_item);
>>> total_bytes = btrfs_device_total_bytes(eb, dev_item);
>>>
>>> + if (used > total_bytes) {
>>> + error("device %llu has incorrect used bytes %llu > total bytes
>>> %llu",
>>> + dev_id, used, total_bytes);
>>> + return ACCOUNTING_MISMATCH;
>>
>> The message and return code point at an error in accounting logic.
>
> One of the biggest problem in lowmem is we don't always have the error
> code we really want.
>
> And that's the case for this error message.
> It's indeed not an accounting error, as in that case (just like that
> test case image) the used bytes is correct accounted.
>
> The good news is, the return value is never really used to classify the
> error.
> So as long as the error message makes sense, it's not a big problem.
Aha. Clear, thanks for explaining.
>
> Thanks,
> Qu
>
>>
>> However, if you have a fully allocated device and a DUP chunk ending
>> beyond device, then having used > total_bytes is expected...
>>
>> So maybe there's two possibilities... There's an error in the accounting
>> logic, or there's an "over-allocation", which is another type of issue
>> which produces used > total with correct accounting logic.
>>
>>> + }
>>> key.objectid = dev_id;
>>> key.type = BTRFS_DEV_EXTENT_KEY;
>>> key.offset = 0;
>>>
>>
>>
>
--
Hans van Kranenburg
Re: [PATCH v3 4/6] btrfs-progs: lowmem check: Add dev_item check for used bytes and total bytes
On 2018/10/9 上午6:20, Hans van Kranenburg wrote:
> On 10/08/2018 02:30 PM, Qu Wenruo wrote:
>> Obviously, used bytes can't be larger than total bytes.
>>
>> Signed-off-by: Qu Wenruo
>> ---
>> check/mode-lowmem.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 07c03cad77af..1173b963b8f3 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -4074,6 +4074,11 @@ static int check_dev_item(struct btrfs_fs_info
>> *fs_info,
>> used = btrfs_device_bytes_used(eb, dev_item);
>> total_bytes = btrfs_device_total_bytes(eb, dev_item);
>>
>> +if (used > total_bytes) {
>> +error("device %llu has incorrect used bytes %llu > total bytes
>> %llu",
>> +dev_id, used, total_bytes);
>> +return ACCOUNTING_MISMATCH;
>
> The message and return code point at an error in accounting logic.
One of the biggest problem in lowmem is we don't always have the error
code we really want.
And that's the case for this error message.
It's indeed not an accounting error, as in that case (just like that
test case image) the used bytes is correct accounted.
The good news is, the return value is never really used to classify the
error.
So as long as the error message makes sense, it's not a big problem.
Thanks,
Qu
>
> However, if you have a fully allocated device and a DUP chunk ending
> beyond device, then having used > total_bytes is expected...
>
> So maybe there's two possibilities... There's an error in the accounting
> logic, or there's an "over-allocation", which is another type of issue
> which produces used > total with correct accounting logic.
>
>> +}
>> key.objectid = dev_id;
>> key.type = BTRFS_DEV_EXTENT_KEY;
>> key.offset = 0;
>>
>
>
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3 4/6] btrfs-progs: lowmem check: Add dev_item check for used bytes and total bytes
On 10/08/2018 02:30 PM, Qu Wenruo wrote:
> Obviously, used bytes can't be larger than total bytes.
>
> Signed-off-by: Qu Wenruo
> ---
> check/mode-lowmem.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 07c03cad77af..1173b963b8f3 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -4074,6 +4074,11 @@ static int check_dev_item(struct btrfs_fs_info
> *fs_info,
> used = btrfs_device_bytes_used(eb, dev_item);
> total_bytes = btrfs_device_total_bytes(eb, dev_item);
>
> + if (used > total_bytes) {
> + error("device %llu has incorrect used bytes %llu > total bytes
> %llu",
> + dev_id, used, total_bytes);
> + return ACCOUNTING_MISMATCH;
The message and return code point at an error in accounting logic.
However, if you have a fully allocated device and a DUP chunk ending
beyond device, then having used > total_bytes is expected...
So maybe there's two possibilities... There's an error in the accounting
logic, or there's an "over-allocation", which is another type of issue
which produces used > total with correct accounting logic.
> + }
> key.objectid = dev_id;
> key.type = BTRFS_DEV_EXTENT_KEY;
> key.offset = 0;
>
--
Hans van Kranenburg
[PATCH v3 4/6] btrfs-progs: lowmem check: Add dev_item check for used bytes and total bytes
Obviously, used bytes can't be larger than total bytes.
Signed-off-by: Qu Wenruo
---
check/mode-lowmem.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 07c03cad77af..1173b963b8f3 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -4074,6 +4074,11 @@ static int check_dev_item(struct btrfs_fs_info *fs_info,
used = btrfs_device_bytes_used(eb, dev_item);
total_bytes = btrfs_device_total_bytes(eb, dev_item);
+ if (used > total_bytes) {
+ error("device %llu has incorrect used bytes %llu > total bytes
%llu",
+ dev_id, used, total_bytes);
+ return ACCOUNTING_MISMATCH;
+ }
key.objectid = dev_id;
key.type = BTRFS_DEV_EXTENT_KEY;
key.offset = 0;
--
2.19.1
