Dave Chinner wrote on 2015/09/30 14:20 +1000:
On Wed, Sep 30, 2015 at 09:05:15AM +0800, Qu Wenruo wrote:
Dave Chinner wrote on 2015/09/30 07:51 +1000:
On Tue, Sep 29, 2015 at 05:34:24PM +0800, Qu Wenruo wrote:
Normally, a bull fallocate call on a fully written and synced file
should not add an extent.
Why not? Filesystems can do whatever they want with extents during
a fallocate call. e.g. if the blocks are shared, then fallocate
might break the block sharing so future overwrites don't get
ENOSPC. This is a requirement set down by posix_fallocate(3)
"After a successful call to posix_fallocate(), subsequent writes to
bytes in the specified range are guaranteed not to fail because of
lack of disk space."
Hence if you've got a file with shared blocks, a "full fallocate"
must change the extent layout to break the sharing. As such, the
premise of this test is wrong.
First, btrfs never meets the posix_fallocate requirement by its COW nature.
Btrfs fallocate can only ensure at most next write will not cause ENOSPC.
Which, it can be successfully argued, meets the basic requirement of
posix_fallocate(). i.e. "subsequent writes" is "one or more" future
writes.
But in trying to explain how COW works you've completely missed the
point I was making about a fundamental principle that COW is based
on - overwrite requires allocation. Hence fallocate must be allowed
modify the underlying layout of a file, even if the file is already
full of allocated blocks and data.
This isn't just btrfs - any filesystem that does dedupe or reflink
or snapshots or compression or any other sort of operation that can
cause extent reallocation on overwrite *may* change the file layout
during a fallocate call to guarantee the next write succeeds.
It's OK not to consider it as a bug, at least data is not corrupted.
But IMO the btrfs behavior is not needed and need optimization.
So kernel patch is submitted to btrfs ml:
https://patchwork.kernel.org/patch/7284431/
That's a link to the fstests patch, not a btrfs kernel patch. :/
Sorry, the real patch is https://patchwork.kernel.org/patch/7283461/
And if fstests is not the proper place, any idea where such "test
case" should belong?
You still haven't understood what I said. If you want to test that
btrfs does not truncate extents beyond EOF when fallocate is called,
then it's a btrfs test. Yes, You can do whatever you want with
btrfs, but you've proposed a generic test that applies a constraint
to a generic operation that has no such constraint defined in it's
API. If you want to constrain fallocate behaviour like this, then
take it to linux-fsdevel and get everyone to agree on the
constraint, and then I'll take it as a generic test...
Now, I think I understand the point.
The test case is not for generic, but for Btrfs only.
The reason is:
Fallocate should be able to change extent layout
Even all extent is allocated, for its purpose.
So the assumption for the test case is not valid, and how a filesystem
handle the last page truncate should follow its own standard.
This is convincing now.
I'll change it to btrfs test.
BTW, at least xfs doesn't truncate beyond EOF, will xfs also need such test?
Thanks,
Qu
Cheers,
Dave.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
