| Message ID | 20181008123044.13413-5-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: check: Detect invalid dev extents and device items | expand |
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 <wqu@suse.com> > --- > 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; >
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 <wqu@suse.com> >> --- >> 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; >> > >
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 <wqu@suse.com> >>> --- >>> 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; >>> >> >> >
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;
Obviously, used bytes can't be larger than total bytes. Signed-off-by: Qu Wenruo <wqu@suse.com> --- check/mode-lowmem.c | 5 +++++ 1 file changed, 5 insertions(+)