[LTP] [PATCH] syscalls/statx01: loosen the stx_blocks check

[LTP] [PATCH] syscalls/statx01: loosen the stx_blocks check

[Steve Muckle]

A filesystem may preallocate blocks (ext4 does this), so even though the
contents of the file may fit into a single block, statx may report more
than one block. Loosen the test to accommodate this.

Signed-off-by: Steve Muckle <smuckle@google.com>
---
 testcases/kernel/syscalls/statx/statx01.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/statx/statx01.c b/testcases/kernel/syscalls/statx/statx01.c
index 23ea99b4a..5e705dc7d 100644
--- a/testcases/kernel/syscalls/statx/statx01.c
+++ b/testcases/kernel/syscalls/statx/statx01.c
@@ -92,7 +92,7 @@ static void test_normal_file(void)
 			buff.stx_mode, MODE);
 
 
-	if (buff.stx_blocks <= buff.stx_blksize/512)
+	if (buff.stx_blocks <= 128)
 		tst_res(TPASS, "stx_blocks(%"PRIu64") is valid",
 			buff.stx_blocks);
 	else
-- 
2.22.0.657.g960e92d24f-goog

[LTP] [PATCH] syscalls/statx01: loosen the stx_blocks check

[Cyril Hrubis]

Hi!
> A filesystem may preallocate blocks (ext4 does this), so even though the
> contents of the file may fit into a single block, statx may report more
> than one block. Loosen the test to accommodate this.
> 
> Signed-off-by: Steve Muckle <smuckle@google.com>
> ---
>  testcases/kernel/syscalls/statx/statx01.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/statx/statx01.c b/testcases/kernel/syscalls/statx/statx01.c
> index 23ea99b4a..5e705dc7d 100644
> --- a/testcases/kernel/syscalls/statx/statx01.c
> +++ b/testcases/kernel/syscalls/statx/statx01.c
> @@ -92,7 +92,7 @@ static void test_normal_file(void)
>  			buff.stx_mode, MODE);
>  
>  
> -	if (buff.stx_blocks <= buff.stx_blksize/512)
> +	if (buff.stx_blocks <= 128)

Why 128?

Can't we rather multiply the buff.stx_blksize/512 by 16?

The filesystem block size is not constant the ppc64le has page size and
block size 64k,@least for Btrfs, so 128 would be a single block
there so if we preallocated blocks there this would stil fail.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/statx01: loosen the stx_blocks check

[Steve Muckle]

On 7/23/19 4:03 AM, Cyril Hrubis wrote:
> Hi!
>> A filesystem may preallocate blocks (ext4 does this), so even though the
>> contents of the file may fit into a single block, statx may report more
>> than one block. Loosen the test to accommodate this.
>>
>> Signed-off-by: Steve Muckle <smuckle@google.com>
>> ---
>>   testcases/kernel/syscalls/statx/statx01.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/testcases/kernel/syscalls/statx/statx01.c b/testcases/kernel/syscalls/statx/statx01.c
>> index 23ea99b4a..5e705dc7d 100644
>> --- a/testcases/kernel/syscalls/statx/statx01.c
>> +++ b/testcases/kernel/syscalls/statx/statx01.c
>> @@ -92,7 +92,7 @@ static void test_normal_file(void)
>>   			buff.stx_mode, MODE);
>>   
>>   
>> -	if (buff.stx_blocks <= buff.stx_blksize/512)
>> +	if (buff.stx_blocks <= 128)
> 
> Why 128?

Was just restoring it to what it was previously.

> Can't we rather multiply the buff.stx_blksize/512 by 16?

That would work for ext4, but should this be loosened further to give 
some headroom in case some FS preallocates more? Or is it preferable to 
keep the test as strict as possible and loosen it when/if failures happen?

[LTP] [PATCH] syscalls/statx01: loosen the stx_blocks check

[Cyril Hrubis]

Hi!
> > Why 128?
> 
> Was just restoring it to what it was previously.
> 
> > Can't we rather multiply the buff.stx_blksize/512 by 16?
> 
> That would work for ext4, but should this be loosened further to give 
> some headroom in case some FS preallocates more? Or is it preferable to 
> keep the test as strict as possible and loosen it when/if failures happen?

That is hard to decide. I would go for the lowest limit that works for
everybody.

Also for me ext4 allocates only one block for the file no matter what I
do, something has to be tuned differently in your setup.

Isn't it just a difference in the block size and inode size?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/statx01: loosen the stx_blocks check

[Steve Muckle]

On 7/24/19 4:57 AM, Cyril Hrubis wrote:
> Hi!
>>> Why 128?
>>
>> Was just restoring it to what it was previously.
>>
>>> Can't we rather multiply the buff.stx_blksize/512 by 16?
>>
>> That would work for ext4, but should this be loosened further to give
>> some headroom in case some FS preallocates more? Or is it preferable to
>> keep the test as strict as possible and loosen it when/if failures happen?
> 
> That is hard to decide. I would go for the lowest limit that works for
> everybody.
> 
> Also for me ext4 allocates only one block for the file no matter what I
> do, something has to be tuned differently in your setup.
> 
> Isn't it just a difference in the block size and inode size?

On my platform stx_blocks = 16 (8kb) and stx_blksize = 4096, so two fs 
blocks are being allocated.

Interestingly, if I create a 256 byte file on this Android platform in 
/data, it 4kb:

vsoc_x86:/data # dd if=/dev/zero of=test_file bs=1 count=256
256+0 records in
256+0 records out
256 bytes (256 B) copied, 0.000926 s, 270 K/s
vsoc_x86:/data # ls -ls test_file
4 -rw-rw-rw- 1 root root 256 2019-07-24 19:28 test_file

However if I go into the directory specified in TMPDIR, it takes 8kb:

vsoc_x86:/data/local/tmp # dd if=/dev/zero of=test_file bs=1 count=256
256+0 records in
256+0 records out
256 bytes (256 B) copied, 0.000927 s, 270 K/s
vsoc_x86:/data/local/tmp # ls -ls
total 8
8 -rw-rw-rw- 1 root root 256 2019-07-24 19:30 test_file

It is also 8kb if I create it in /data/local, but not if I create a 
directory at /data/testdir and create the file there.

Perhaps the x-attributes are spilling out of the inode in some cases and 
into a data block? I don't know enough about filesystems to say.

[LTP] [PATCH] syscalls/statx01: loosen the stx_blocks check

[Cyril Hrubis]

Hi!
> >> That would work for ext4, but should this be loosened further to give
> >> some headroom in case some FS preallocates more? Or is it preferable to
> >> keep the test as strict as possible and loosen it when/if failures happen?
> > 
> > That is hard to decide. I would go for the lowest limit that works for
> > everybody.
> > 
> > Also for me ext4 allocates only one block for the file no matter what I
> > do, something has to be tuned differently in your setup.
> > 
> > Isn't it just a difference in the block size and inode size?
> 
> On my platform stx_blocks = 16 (8kb) and stx_blksize = 4096, so two fs 
> blocks are being allocated.
> 
> Interestingly, if I create a 256 byte file on this Android platform in 
> /data, it 4kb:
> 
> vsoc_x86:/data # dd if=/dev/zero of=test_file bs=1 count=256
> 256+0 records in
> 256+0 records out
> 256 bytes (256 B) copied, 0.000926 s, 270 K/s
> vsoc_x86:/data # ls -ls test_file
> 4 -rw-rw-rw- 1 root root 256 2019-07-24 19:28 test_file
> 
> However if I go into the directory specified in TMPDIR, it takes 8kb:
> 
> vsoc_x86:/data/local/tmp # dd if=/dev/zero of=test_file bs=1 count=256
> 256+0 records in
> 256+0 records out
> 256 bytes (256 B) copied, 0.000927 s, 270 K/s
> vsoc_x86:/data/local/tmp # ls -ls
> total 8
> 8 -rw-rw-rw- 1 root root 256 2019-07-24 19:30 test_file
> 
> It is also 8kb if I create it in /data/local, but not if I create a 
> directory at /data/testdir and create the file there.

That is a bit strange, unfortunately I do not have a single Android
device around, otherwise I would try to reproduce it.

> Perhaps the x-attributes are spilling out of the inode in some cases and 
> into a data block? I don't know enough about filesystems to say.

That may be, you can try to list these with getfattr.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/statx01: loosen the stx_blocks check

[Steve Muckle]

On 7/25/19 5:48 AM, Cyril Hrubis wrote:
>> Perhaps the x-attributes are spilling out of the inode in some cases and
>> into a data block? I don't know enough about filesystems to say.
> 
> That may be, you can try to list these with getfattr.

This turned out to be because of ext4 per-file encryption which adds 
another x-attr, possibly causing the allocation of another block for 
xattrs. This wasn't apparent initially because the encryption xattr is 
hidden and not reported by getfattr.

I'll send out an updated patch.