| Message ID | 20240905134502.33759-2-mdoucha@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] fallocate05: Check that deallocated file range is marked as a hole | expand |
On Thu, Sep 5, 2024 at 9:45 PM Martin Doucha <mdoucha@suse.cz> wrote: > The default deallocation size is likely too small for bcachefs > to actually release the blocks. Since it is also a copy-on-write > filesystem, deallocated the whole file like on Btrfs. > > Signed-off-by: Martin Doucha <mdoucha@suse.cz> --- > > AFAICT Bcachefs uses 512 byte data blocks by default but 256KB inode > blocks. The whole file will be 128KB and 32KB gets deallocated which may > be too small. However, I'm not entirely sure whether this is the best > solution. > I think this solution is correct. Even deallocating a hole size (32KB) that is larger than a block size 512bytes, but that does not mean the Bcachefs can satisfy enough size for following written behavior, because the allocation of new data blocks (hole) might involve modifying metadata structures that need more space(extends more inode blocks 256KB). If the filesystem cannot accommodate these changes, it could lead to an ENOSPC error. Reviewed-by: Li Wang <liwang@redhat.com>
Hi Martin, all, [ Cc Kent and Bcachefs ML ] > The default deallocation size is likely too small for bcachefs > to actually release the blocks. Since it is also a copy-on-write > filesystem, deallocated the whole file like on Btrfs. Make sense. Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr > Signed-off-by: Martin Doucha <mdoucha@suse.cz> > --- > AFAICT Bcachefs uses 512 byte data blocks by default but 256KB inode > blocks. The whole file will be 128KB and 32KB gets deallocated which may > be too small. However, I'm not entirely sure whether this is the best > solution. > See also https://bugzilla.suse.com/show_bug.cgi?id=1230155 > testcases/kernel/syscalls/fallocate/fallocate05.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > diff --git a/testcases/kernel/syscalls/fallocate/fallocate05.c b/testcases/kernel/syscalls/fallocate/fallocate05.c > index 979c70d6e..732a2f15d 100644 > --- a/testcases/kernel/syscalls/fallocate/fallocate05.c > +++ b/testcases/kernel/syscalls/fallocate/fallocate05.c > @@ -114,7 +114,8 @@ static void run(void) > tst_res(TPASS, "fallocate() on full FS"); > /* Btrfs deallocates only complete extents, not individual blocks */ > - if (!strcmp(tst_device->fs_type, "btrfs")) > + if (!strcmp(tst_device->fs_type, "btrfs") || > + !strcmp(tst_device->fs_type, "bcachefs")) > holesize = bufsize + extsize; > else > holesize = DEALLOCATE_BLOCKS * blocksize;
On Fri, Sep 06, 2024 at 10:02:00AM GMT, Petr Vorel wrote: > Hi Martin, all, > > [ Cc Kent and Bcachefs ML ] > > > The default deallocation size is likely too small for bcachefs > > to actually release the blocks. Since it is also a copy-on-write > > filesystem, deallocated the whole file like on Btrfs. > > Make sense. > Reviewed-by: Petr Vorel <pvorel@suse.cz> I haven't looked at this specific test, but one thing that we run into with the weird CoW behaviour tests in xfstests is that bcachefs btree nodes are 256k by default - you're going to see weird effects from that if tests are looking at disk usage.
On Fri, Sep 6, 2024 at 6:18 PM Kent Overstreet <kent.overstreet@linux.dev> wrote: > On Fri, Sep 06, 2024 at 10:02:00AM GMT, Petr Vorel wrote: > > Hi Martin, all, > > > > [ Cc Kent and Bcachefs ML ] > > > > > The default deallocation size is likely too small for bcachefs > > > to actually release the blocks. Since it is also a copy-on-write > > > filesystem, deallocated the whole file like on Btrfs. > > > > Make sense. > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > I haven't looked at this specific test, but one thing that we run into > with the weird CoW behaviour tests in xfstests is that bcachefs btree > nodes are 256k by default - you're going to see weird effects from that > if tests are looking at disk usage. > Thank you. Patch merged.
diff --git a/testcases/kernel/syscalls/fallocate/fallocate05.c b/testcases/kernel/syscalls/fallocate/fallocate05.c index 979c70d6e..732a2f15d 100644 --- a/testcases/kernel/syscalls/fallocate/fallocate05.c +++ b/testcases/kernel/syscalls/fallocate/fallocate05.c @@ -114,7 +114,8 @@ static void run(void) tst_res(TPASS, "fallocate() on full FS"); /* Btrfs deallocates only complete extents, not individual blocks */ - if (!strcmp(tst_device->fs_type, "btrfs")) + if (!strcmp(tst_device->fs_type, "btrfs") || + !strcmp(tst_device->fs_type, "bcachefs")) holesize = bufsize + extsize; else holesize = DEALLOCATE_BLOCKS * blocksize;
The default deallocation size is likely too small for bcachefs to actually release the blocks. Since it is also a copy-on-write filesystem, deallocated the whole file like on Btrfs. Signed-off-by: Martin Doucha <mdoucha@suse.cz> --- AFAICT Bcachefs uses 512 byte data blocks by default but 256KB inode blocks. The whole file will be 128KB and 32KB gets deallocated which may be too small. However, I'm not entirely sure whether this is the best solution. See also https://bugzilla.suse.com/show_bug.cgi?id=1230155 testcases/kernel/syscalls/fallocate/fallocate05.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)