Re: [PATCH] btrfs: tests: Use BTRFS_MAX_EXTENT_SIZE to replace the intermediate number

2018-11-03 Thread Nikolay Borisov



On 3.11.18 г. 11:24 ч., Qu Wenruo wrote:
> In extent-io self test, we need 2 ordered extents at its maximum size to
> do the test.
> 
> Instead of using the intermediate numbers, use BTRFS_MAX_EXTENT_SIZE for
> @max_bytes, and twice @max_bytes for @total_dirty.
> This should explain why we need all these magic numbers and prevent
> people to modify them by accident.
> 
> Signed-off-by: Qu Wenruo 

Reviewed-by: Nikolay Borisov 

> ---
>  fs/btrfs/tests/extent-io-tests.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/tests/extent-io-tests.c 
> b/fs/btrfs/tests/extent-io-tests.c
> index 9e0f4a01be14..c0ad9bc48f6b 100644
> --- a/fs/btrfs/tests/extent-io-tests.c
> +++ b/fs/btrfs/tests/extent-io-tests.c
> @@ -62,8 +62,9 @@ static int test_find_delalloc(u32 sectorsize)
>   struct page *page;
>   struct page *locked_page = NULL;
>   unsigned long index = 0;
> - u64 total_dirty = SZ_256M;
> - u64 max_bytes = SZ_128M;
> + /* In this test we need at least 2 file extents at its maximum size */
> + u64 max_bytes = BTRFS_MAX_EXTENT_SIZE;
> + u64 total_dirty = 2 * max_bytes;
>   u64 start, end, test_start;
>   u64 found;
>   int ret = -EINVAL;
> 


[PATCH] btrfs: tests: Use BTRFS_MAX_EXTENT_SIZE to replace the intermediate number

2018-11-03 Thread Qu Wenruo
In extent-io self test, we need 2 ordered extents at its maximum size to
do the test.

Instead of using the intermediate numbers, use BTRFS_MAX_EXTENT_SIZE for
@max_bytes, and twice @max_bytes for @total_dirty.
This should explain why we need all these magic numbers and prevent
people to modify them by accident.

Signed-off-by: Qu Wenruo 
---
 fs/btrfs/tests/extent-io-tests.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index 9e0f4a01be14..c0ad9bc48f6b 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -62,8 +62,9 @@ static int test_find_delalloc(u32 sectorsize)
struct page *page;
struct page *locked_page = NULL;
unsigned long index = 0;
-   u64 total_dirty = SZ_256M;
-   u64 max_bytes = SZ_128M;
+   /* In this test we need at least 2 file extents at its maximum size */
+   u64 max_bytes = BTRFS_MAX_EXTENT_SIZE;
+   u64 total_dirty = 2 * max_bytes;
u64 start, end, test_start;
u64 found;
int ret = -EINVAL;
-- 
2.19.1



Re: fsck lowmem mode only: ERROR: errors found in fs roots

2018-11-03 Thread Nikolay Borisov



On 3.11.18 г. 3:34 ч., Su Yue wrote:
> 
> 
> On 2018/11/2 10:10 PM, Christoph Anton Mitterer wrote:
>> Hey Su.
>>
> 
> Sorry for the late reply cause I'm busy at other things.
> 
>> Anything further I need to do in this matter or can I consider it
>> "solved" and you won't need further testing by my side, but just PR the
>> patches of that branch? :-)
>>
> 
> I just looked through related codes and found the bug.
> The patches can fix it. So no need to do more tests.
> Thanks to your tests and patience. :)
> 
> 
> In previous output of debug version, we can see @ret code
> is 524296 which is (DIR_ITEM_MISMATCH(1 << 3) | DIR_INDEX_MISMATCH
> (1<<19)).
> 
> In btrfs-progs v4.17,
> function check_inode_extref() passes u64 @mode as the last parameter
> of find_dir_item();
> However, find_dir_item() is defined as:
> static int find_dir_item(struct btrfs_root *root, struct btrfs_key *key,
>  struct btrfs_key *location_key, char *name,
>  u32 namelen, u8 file_type);
> 
> The type of the last argument is u8 not u64.

So this would have been caught by gcc's -Wconversion, except it likely
wouldn't have been because right now this option produces loads of false
positives... Too bad...

> 
> So the case is that while checking files with inode_extrefs,
> if (imode != file_type), then find_dir_item() thinks it found
> DIR_ITEM_MISMATCH or DIR_INDEX_MISMATCH.
> 
> Thanks,
> Su
> 
>> Thanks,
>> Chris.
>>
>> On Sat, 2018-10-27 at 14:15 +0200, Christoph Anton Mitterer wrote:
>>> Hey.
>>>
>>>
>>> Without the last patches on 4.17:
>>>
>>> checking extents
>>> checking free space cache
>>> checking fs roots
>>> ERROR: errors found in fs roots
>>> Checking filesystem on /dev/mapper/system
>>> UUID: 6050ca10-e778-4d08-80e7-6d27b9c89b3c
>>> found 619543498752 bytes used, error(s) found
>>> total csum bytes: 602382204
>>> total tree bytes: 2534309888
>>> total fs tree bytes: 1652097024
>>> total extent tree bytes: 160432128
>>> btree space waste bytes: 459291608
>>> file data blocks allocated: 7334036647936
>>>   referenced 730839187456
>>>
>>>
>>> With the last patches, on 4.17:
>>>
>>> checking extents
>>> checking free space cache
>>> checking fs roots
>>> checking only csum items (without verifying data)
>>> checking root refs
>>> Checking filesystem on /dev/mapper/system
>>> UUID: 6050ca10-e778-4d08-80e7-6d27b9c89b3c
>>> found 619543498752 bytes used, no error found
>>> total csum bytes: 602382204
>>> total tree bytes: 2534309888
>>> total fs tree bytes: 1652097024
>>> total extent tree bytes: 160432128
>>> btree space waste bytes: 459291608
>>> file data blocks allocated: 7334036647936
>>>   referenced 730839187456
>>>
>>>
>>> Cheers,
>>> Chris.
>>>
>>


Re: BTRFS did it's job nicely (thanks!)

2018-11-03 Thread Duncan
waxhead posted on Fri, 02 Nov 2018 20:54:40 +0100 as excerpted:

> Note that I tend to interpret the btrfs de st / output as if the error
> was NOT fixed even if (seems clearly that) it was, so I think the output
> is a bit misleading... just saying...

See the btrfs-device manpage, stats subcommand, -z|--reset option, and 
device stats section:

-z|--reset
Print the stats and reset the values to zero afterwards.

DEVICE STATS
The device stats keep persistent record of several error classes related 
to doing IO. The current values are printed at mount time and
updated during filesystem lifetime or from a scrub run.


So stats keeps a count of historic errors and is only reset when you 
specifically reset it, *NOT* when the error is fixed.

(There's actually a recent patch, I believe in the current dev kernel 
4.20/5.0, that will reset a device's stats automatically for the btrfs 
replace case when it's actually a different device afterward anyway.  
Apparently, it doesn't even do /that/ automatically yet.  Keep that in 
mind if you replace that device.)

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman