syscalls/fallocate04: Fix on Btrfs

PATCH New Cyril Hrubis 3 participants 7 messages Aug 25, 2016 – Aug 31, 2016

Share & tools

Cyril Hrubis

author

2016-08-25 16:53 UTC #0 lore raw

PATCH 1/1 syscalls/fallocate04: Fix on Btrfs

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 <[email protected]>

---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 2 ++
 1 file changed, 2 insertions(+)

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -0

index 2b904e4..f204188 100644
@@ -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;

Signature · 6 lines

2.7.3


-- 
Cyril Hrubis
[email protected]

Stanislav Kholmanskikh

2016-08-26 11:17 UTC #1 lore raw

PATCH 1/1 syscalls/fallocate04: Fix on Btrfs

Hi,

Quoted 13 lines · 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:

Quoted 9 lines

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:

a 4.1-based kernel:

* 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.

Quoted 19 lines

Signed-off-by Cyril Hrubis <[email protected]>

---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 2 ++
 1 file changed, 2 insertions(+)

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -0

index 2b904e4..f204188 100644
@@ -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;

Alexey Kodanev

2016-08-26 12:08 UTC #2 lore raw

PATCH 1/1 syscalls/fallocate04: Fix on Btrfs

Hi,

Quoted 8 lines · Stanislav Kholmanskikh

Hi,
Quoted 5 lines · 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.

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

Cyril Hrubis

author

2016-08-29 09:44 UTC #3 lore raw

PATCH 1/1 syscalls/fallocate04: Fix on Btrfs

Hi!

Quoted 8 lines

Quoted 5 lines
Quoted 5 lines
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...

Signature · 2 lines

Cyril Hrubis
[email protected]

Alexey Kodanev

2016-08-29 12:16 UTC #4 lore raw

PATCH 1/1 syscalls/fallocate04: Fix on Btrfs

Hi,

Quoted 6 lines · Cyril Hrubis

Hi!
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
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.

Quoted 8 lines

Quoted 2 lines
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.

Quoted 4 lines

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

Cyril Hrubis

author

2016-08-29 12:36 UTC #5 lore raw

PATCH 1/1 syscalls/fallocate04: Fix on Btrfs

Hi!

Quoted 4 lines

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.

Quoted 2 lines

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.

Quoted 6 lines

Quoted 4 lines
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.

Signature · 2 lines

Cyril Hrubis
[email protected]

Cyril Hrubis

author

2016-08-31 14:07 UTC #6 lore raw

PATCH 1/1 syscalls/fallocate04: Fix on Btrfs

In reply to Cyril Hrubis

Hi!
Patch pushed.

Signature · 2 lines

Cyril Hrubis
[email protected]