[LTP] [PATCH] syscalls/fallocate04: Fix on Btrfs

[LTP] [PATCH] syscalls/fallocate04: Fix on Btrfs

[Cyril Hrubis]

The size of a file can double on Btrfs temporarily even when we write
into a preallocated space because of internal Btrfs caches, at least
that is what I've been told by Btrfs devs. The internal caches are then
freed after some time, or can be force freed by syncing the file, which
is what this patch does.

Technically we are not guaranteed that the file size will not change after
writing to preallocated space. We are only guaranteed that subsequent writes to
the region will not fail with ENOSPC. So this behavior, while uncommon, is not
really a bug.

Before the patch the test fails with:

fallocate04    0  TINFO  :  allocate '12288' bytes
fallocate04    1  TPASS  :  test-case succeeded
fallocate04    0  TINFO  :  read allocated file size '24576'
fallocate04    0  TINFO  :  make a hole with FALLOC_FL_PUNCH_HOLE
fallocate04    0  TINFO  :  check that file has a hole with lseek(,,SEEK_HOLE)
fallocate04    0  TINFO  :  found a hole at '4096' offset
fallocate04    0  TINFO  :  allocated file size before '24576' and after '12288'
fallocate04    2  TFAIL  :  fallocate04.c:184: not expected allocated size

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
index 2b904e4..f204188 100644
--- a/testcases/kernel/syscalls/fallocate/fallocate04.c
+++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
@@ -75,6 +75,8 @@ static size_t get_allocsize(void)
 {
 	struct stat file_stat;
 
+	fsync(fd);
+
 	SAFE_FSTAT(cleanup, fd, &file_stat);
 
 	return file_stat.st_blocks * 512;
-- 
2.7.3


-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/fallocate04: Fix on Btrfs

[Stanislav Kholmanskikh]

Hi,

On 08/25/2016 07:53 PM, Cyril Hrubis wrote:
> The size of a file can double on Btrfs temporarily even when we write
> into a preallocated space because of internal Btrfs caches, at least
> that is what I've been told by Btrfs devs. The internal caches are then
> freed after some time, or can be force freed by syncing the file, which
> is what this patch does.
> 
> Technically we are not guaranteed that the file size will not change after
> writing to preallocated space. We are only guaranteed that subsequent writes to
> the region will not fail with ENOSPC. So this behavior, while uncommon, is not
> really a bug.
> 
> Before the patch the test fails with:
> 
> fallocate04    0  TINFO  :  allocate '12288' bytes
> fallocate04    1  TPASS  :  test-case succeeded
> fallocate04    0  TINFO  :  read allocated file size '24576'
> fallocate04    0  TINFO  :  make a hole with FALLOC_FL_PUNCH_HOLE
> fallocate04    0  TINFO  :  check that file has a hole with lseek(,,SEEK_HOLE)
> fallocate04    0  TINFO  :  found a hole at '4096' offset
> fallocate04    0  TINFO  :  allocated file size before '24576' and after '12288'
> fallocate04    2  TFAIL  :  fallocate04.c:184: not expected allocated size
> 

Tested this on SPARC:

* without the patch

a 2.6.39-based kernel:

[root@skholman-m7 fallocate]# TMPDIR=/mnt ./fallocate04
fallocate04 0 TINFO : allocate '24576' bytes
fallocate04 1 TPASS : test-case succeeded
fallocate04 0 TINFO : read allocated file size '49152'
fallocate04 0 TINFO : make a hole with FALLOC_FL_PUNCH_HOLE
fallocate04 0 TINFO : check that file has a hole with lseek(,,SEEK_HOLE)
fallocate04 0 TINFO : lseek() doesn't support SEEK_HOLE, this is
expected for < 3.1 kernels
fallocate04 0 TINFO : allocated file size before '49152' and after '24576'
fallocate04 2 TFAIL : fallocate04.c:184: not expected allocated size
[root@skholman-m7 fallocate]#

a 4.1-based kernel:

[root@skholman-m7 fallocate]# TMPDIR=/mnt ./fallocate04
fallocate04 0 TINFO : allocate '24576' bytes
fallocate04 1 TPASS : test-case succeeded
fallocate04 0 TINFO : read allocated file size '49152'
fallocate04 0 TINFO : make a hole with FALLOC_FL_PUNCH_HOLE
fallocate04 0 TINFO : check that file has a hole with lseek(,,SEEK_HOLE)
fallocate04 0 TINFO : found a hole at '8192' offset
fallocate04 0 TINFO : allocated file size before '49152' and after '24576'
fallocate04 2 TFAIL : fallocate04.c:184: not expected allocated size
[root@skholman-m7 fallocate]#

* with your patch

a 2.6.39-based kernel:

[root@skholman-m7 fallocate]# TMPDIR=/mnt ./fallocate04
fallocate04 0 TINFO : allocate '24576' bytes
fallocate04 1 TPASS : test-case succeeded
fallocate04 0 TINFO : read allocated file size '24576'
fallocate04 0 TINFO : make a hole with FALLOC_FL_PUNCH_HOLE
fallocate04 0 TINFO : check that file has a hole with lseek(,,SEEK_HOLE)
fallocate04 0 TINFO : lseek() doesn't support SEEK_HOLE, this is
expected for < 3.1 kernels
fallocate04 0 TINFO : allocated file size before '24576' and after '16384'
fallocate04 0 TINFO : reading the file, compare with expected buffer
fallocate04 2 TPASS : test-case succeeded
fallocate04 0 TINFO : zeroing file space with FALLOC_FL_ZERO_RANGE
fallocate04 3 TCONF : fallocate04.c:204: FALLOC_FL_ZERO_RANGE needs
Linux 3.15 or newer
fallocate04 4 TCONF : fallocate04.c:204: Remaining cases not appropriate
for configuration
[root@skholman-m7 fallocate]#

a 4.1-based kernel:

[root@skholman-m7 fallocate]# TMPDIR=/mnt ./fallocate04
fallocate04 0 TINFO : allocate '24576' bytes
fallocate04 1 TPASS : test-case succeeded
fallocate04 0 TINFO : read allocated file size '24576'
fallocate04 0 TINFO : make a hole with FALLOC_FL_PUNCH_HOLE
fallocate04 0 TINFO : check that file has a hole with lseek(,,SEEK_HOLE)
fallocate04 0 TINFO : found a hole@'8192' offset
fallocate04 0 TINFO : allocated file size before '24576' and after '16384'
fallocate04 0 TINFO : reading the file, compare with expected buffer
fallocate04 2 TPASS : test-case succeeded
fallocate04 0 TINFO : zeroing file space with FALLOC_FL_ZERO_RANGE
fallocate04 0 TINFO : read current allocated file size '16384'
fallocate04 3 TCONF : fallocate04.c:215: FALLOC_FL_ZERO_RANGE not supported
fallocate04 4 TCONF : fallocate04.c:215: Remaining cases not appropriate
for configuration
[root@skholman-m7 fallocate]#

Thanks for solving this problem.


> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  testcases/kernel/syscalls/fallocate/fallocate04.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
> index 2b904e4..f204188 100644
> --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> @@ -75,6 +75,8 @@ static size_t get_allocsize(void)
>  {
>  	struct stat file_stat;
>  
> +	fsync(fd);
> +
>  	SAFE_FSTAT(cleanup, fd, &file_stat);
>  
>  	return file_stat.st_blocks * 512;
> 

[LTP] [PATCH] syscalls/fallocate04: Fix on Btrfs

[Alexey Kodanev]

Hi,
On 08/26/2016 02:17 PM, Stanislav Kholmanskikh wrote:
> Hi,
>
> On 08/25/2016 07:53 PM, Cyril Hrubis wrote:
>> The size of a file can double on Btrfs temporarily even when we write
>> into a preallocated space because of internal Btrfs caches, at least
>> that is what I've been told by Btrfs devs. The internal caches are then
>> freed after some time, or can be force freed by syncing the file, which
>> is what this patch does.

So if we have available free space x Mb,  we can't allocate more than x 
/ 2 Mb and it's not a bug?

Best regards,
Alexey

[LTP] [PATCH] syscalls/fallocate04: Fix on Btrfs

[Cyril Hrubis]

Hi!
> >> The size of a file can double on Btrfs temporarily even when we write
> >> into a preallocated space because of internal Btrfs caches, at least
> >> that is what I've been told by Btrfs devs. The internal caches are then
> >> freed after some time, or can be force freed by syncing the file, which
> >> is what this patch does.
> 
> So if we have available free space x Mb,  we can't allocate more than x 
> / 2 Mb and it's not a bug?

It's the other way around. We preallocate x Mb for the file, then write
into the preallocated space. The file size, as reported by stat(), may
end up > x MB after the write temporarily on Btrfs because of caching.
And this is not a bug since the only guarantee we have is that we don't
get ENOSPC when writing into preallocated file region. There is nothing
that forbids allocating more space temporarily if there is plenty of it.

Another interesting testcase would be doing the same on the LTP loopback
device that would be filled up after the file has been preallocated.
That would likely force different codepath in the FS since there would
be no avaialable free space left...

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/fallocate04: Fix on Btrfs

[Alexey Kodanev]

Hi,
On 08/29/2016 12:44 PM, Cyril Hrubis wrote:
> Hi!
>>>> The size of a file can double on Btrfs temporarily even when we write
>>>> into a preallocated space because of internal Btrfs caches, at least
>>>> that is what I've been told by Btrfs devs. The internal caches are then
>>>> freed after some time, or can be force freed by syncing the file, which
>>>> is what this patch does.
>> So if we have available free space x Mb,  we can't allocate more than x
>> / 2 Mb and it's not a bug?
> It's the other way around. We preallocate x Mb for the file, then write
> into the preallocated space. The file size, as reported by stat(), may
> end up > x MB after the write temporarily on Btrfs because of caching.
> And this is not a bug since the only guarantee we have is that we don't
> get ENOSPC when writing into preallocated file region. There is nothing
> that forbids allocating more space temporarily if there is plenty of it.

I understand that the size might be larger and we are allocating in 
chunks for
that reason. But here we have "cache" that equals the size of 
allocation...  it just looks suspicious.

It's true, it doesn't matter if we have plenty of space and if we are 
testing
writes that shouldn't return ENOSPC.


> Another interesting testcase would be doing the same on the LTP loopback
> device that would be filled up after the file has been preallocated.
> That would likely force different codepath in the FS since there would
> be no avaialable free space left...

On btrfs it will be hard to do, I mean filling free space :)

Best regards,
Alexey

[LTP] [PATCH] syscalls/fallocate04: Fix on Btrfs

[Cyril Hrubis]

Hi!
> I understand that the size might be larger and we are allocating in 
> chunks for
> that reason. But here we have "cache" that equals the size of 
> allocation...  it just looks suspicious.

I know that this feels like an workaround, but I've been told that it's
OK. My guess is that it has something to do with the COW, it looks like
the whole write is done to a different set of blocks in this case and
that the preallocated blocks are discarded later when Btrfs figures out
that we do not need these. But that is just a guess, I'm not Btrfs
expert and I do not want to spend a week trying to figure out what
exactly is going on here. If you know somebody who can explain this
better, I would be glad to hear it.

> It's true, it doesn't matter if we have plenty of space and if we are 
> testing writes that shouldn't return ENOSPC.

That is the reason why I proposed this patch.

> > Another interesting testcase would be doing the same on the LTP loopback
> > device that would be filled up after the file has been preallocated.
> > That would likely force different codepath in the FS since there would
> > be no avaialable free space left...
> 
> On btrfs it will be hard to do, I mean filling free space :)

We can write to file(s) in a loop until we get ENOSPC, that should be as
close to real out-of-free-space situation as possible.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/fallocate04: Fix on Btrfs

[Cyril Hrubis]

Hi!
Patch pushed.

-- 
Cyril Hrubis
chrubis@suse.cz