| Message ID | 20231207194011.273027-4-pvorel@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Add support bcachefs filesystem | expand |
Hi Kent, On Wed, Dec 13, 2023 at 10:55 AM Kent Overstreet <kent.overstreet@linux.dev> wrote: > On Thu, Dec 07, 2023 at 08:40:11PM +0100, Petr Vorel wrote: > > STATX_ATTR_COMPRESSED is not supported on Bcachefs, thus skip it > > (it's already skipped on tmpfs and XFS). > > Hang on, bcachefs most definitely does hae compression. This would be > just because statx isn't plumbed through? > Seems not, I guess that is only because bcachefs doesn't support the flag in bch2_getattr()? see: https://elixir.bootlin.com/linux/v6.7-rc8/source/fs/bcachefs/fs.c#L741
Hi! > On Thu, Dec 07, 2023 at 08:40:11PM +0100, Petr Vorel wrote: > > STATX_ATTR_COMPRESSED is not supported on Bcachefs, thus skip it > > (it's already skipped on tmpfs and XFS). > > Hang on, bcachefs most definitely does hae compression. This would be > just because statx isn't plumbed through? Quite likely, other filesystems does have an inode flag that is set when file has been compressed and simply report that in the foo_getattr() callback. Looking at bcachefs I supose that we need to figure out if the inode is v3 and then unpack the v3 info to get to the compressed flag, you probably know best how to do that.
On Mon, Jan 08, 2024 at 11:01:40AM +0100, Cyril Hrubis wrote: > Hi! > > On Thu, Dec 07, 2023 at 08:40:11PM +0100, Petr Vorel wrote: > > > STATX_ATTR_COMPRESSED is not supported on Bcachefs, thus skip it > > > (it's already skipped on tmpfs and XFS). > > > > Hang on, bcachefs most definitely does hae compression. This would be > > just because statx isn't plumbed through? > > Quite likely, other filesystems does have an inode flag that is set when > file has been compressed and simply report that in the foo_getattr() > callback. Looking at bcachefs I supose that we need to figure out if the > inode is v3 and then unpack the v3 info to get to the compressed flag, > you probably know best how to do that. I'm still not clear how we want to map STATX_ATTR_COMPRESSED, since it's extents that are compressed, not entire files - and just reporting the compression option is probably not what we want since it can be flipped off, and existing data will still be compressed. Do you know anything about the intended use case?
Hi! > > Quite likely, other filesystems does have an inode flag that is set when > > file has been compressed and simply report that in the foo_getattr() > > callback. Looking at bcachefs I supose that we need to figure out if the > > inode is v3 and then unpack the v3 info to get to the compressed flag, > > you probably know best how to do that. > > I'm still not clear how we want to map STATX_ATTR_COMPRESSED, since it's > extents that are compressed, not entire files - and just reporting the > compression option is probably not what we want since it can be flipped > off, and existing data will still be compressed. > > Do you know anything about the intended use case? As far as I understand the flag it's a hint that the file I/O may be slower/need more memory because of the compression.
Hi! > > > Quite likely, other filesystems does have an inode flag that is set when > > > file has been compressed and simply report that in the foo_getattr() > > > callback. Looking at bcachefs I supose that we need to figure out if the > > > inode is v3 and then unpack the v3 info to get to the compressed flag, > > > you probably know best how to do that. > > > > I'm still not clear how we want to map STATX_ATTR_COMPRESSED, since it's > > extents that are compressed, not entire files - and just reporting the > > compression option is probably not what we want since it can be flipped > > off, and existing data will still be compressed. > > > > Do you know anything about the intended use case? > > As far as I understand the flag it's a hint that the file I/O may be > slower/need more memory because of the compression. Ping. Kent we are getting closer to a next LTP release, are you going to add the compressed flag support into bcachefs or should we add bcachefs to the test skiplist?
diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c index 58296bd24..c2ed52deb 100644 --- a/testcases/kernel/syscalls/statx/statx04.c +++ b/testcases/kernel/syscalls/statx/statx04.c @@ -96,8 +96,9 @@ static void setup(void) for (i = 0, expected_mask = 0; i < ARRAY_SIZE(attr_list); i++) expected_mask |= attr_list[i].attr; - /* STATX_ATTR_COMPRESSED not supported on XFS, TMPFS */ - if (!strcmp(tst_device->fs_type, "xfs") || !strcmp(tst_device->fs_type, "tmpfs")) + /* STATX_ATTR_COMPRESSED not supported on Bcachefs, TMPFS, XFS */ + if (!strcmp(tst_device->fs_type, "bcachefs") || !strcmp(tst_device->fs_type, "tmpfs") || + !strcmp(tst_device->fs_type, "xfs")) expected_mask &= ~STATX_ATTR_COMPRESSED; /* Attribute support was added to Btrfs statx() in kernel v4.13 */
STATX_ATTR_COMPRESSED is not supported on Bcachefs, thus skip it (it's already skipped on tmpfs and XFS). Suggested-by: Cyril Hrubis <chrubis@suse.cz> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- New in v2 testcases/kernel/syscalls/statx/statx04.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)