[PATCH v2 0/3] Add support bcachefs filesystem

[PATCH v2 0/3] Add support bcachefs filesystem

[Petr Vorel]

Hi all,

@Amir @Jan, could you please have look on fanotify failures on Bcachefs?
fanotify13.c, fanotify15.c and fanotify16.c produce many errors.

To test, just apply this patchset and then run with LTP_SINGLE_FS_TYPE:

# LTP_SINGLE_FS_TYPE=bcachefs ./fanotify15

changes v1->v2:
Two new commits:
* lib: Add Bcachefs magic
* statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

Petr Vorel (3):
  lib: Add Bcachefs magic
  lib: Add support bcachefs filesystem to .all_filesystems
  statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

 include/tst_fs.h                          | 3 +++
 lib/tst_fs_type.c                         | 2 ++
 lib/tst_supported_fs_types.c              | 2 ++
 testcases/kernel/syscalls/statx/statx04.c | 5 +++--
 4 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] lib: Add Bcachefs magic

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2

 include/tst_fs.h  | 3 +++
 lib/tst_fs_type.c | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/include/tst_fs.h b/include/tst_fs.h
index eb9841165..06f5be896 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -34,6 +34,9 @@
 #define TST_VFAT_MAGIC     0x4d44 /* AKA MSDOS */
 #define TST_EXFAT_MAGIC    0x2011BAB0UL
 
+/* fs/bcachefs/bcachefs_format.h */
+#define TST_BCACHE_MAGIC		0xca451a4e
+
 enum tst_fill_access_pattern {
 	TST_FILL_BLOCKS,
 	TST_FILL_RANDOM
diff --git a/lib/tst_fs_type.c b/lib/tst_fs_type.c
index d9c9c0817..7f0d61a8e 100644
--- a/lib/tst_fs_type.c
+++ b/lib/tst_fs_type.c
@@ -36,6 +36,8 @@ const char *tst_fs_type_name(long f_type)
 		return "9p";
 	case TST_RAMFS_MAGIC:
 		return "ramfs";
+	case TST_BCACHE_MAGIC:
+		return "bcachefs";
 	case TST_BTRFS_MAGIC:
 		return "btrfs";
 	case TST_XFS_MAGIC:
-- 
2.43.0

Re: [PATCH v2 1/3] lib: Add Bcachefs magic

[Su Yue]

On Thu 07 Dec 2023 at 20:40, Petr Vorel <pvorel@suse.cz> wrote:

> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> New in v2
>
>  include/tst_fs.h  | 3 +++
>  lib/tst_fs_type.c | 2 ++
>  2 files changed, 5 insertions(+)
>
> diff --git a/include/tst_fs.h b/include/tst_fs.h
> index eb9841165..06f5be896 100644
> --- a/include/tst_fs.h
> +++ b/include/tst_fs.h
> @@ -34,6 +34,9 @@
>  #define TST_VFAT_MAGIC     0x4d44 /* AKA MSDOS */
>  #define TST_EXFAT_MAGIC    0x2011BAB0UL
>
> +/* fs/bcachefs/bcachefs_format.h */
> +#define TST_BCACHE_MAGIC		0xca451a4e
> +
>
Not object to the patch. There are three MAGIC values in
bcachefs_format.h: BCACHE_MAGIC, BCHFS_MAGIC, 
BCACHEFS_STATFS_MAGIC.

IIUC, BCACHE_MAGIC is the older one in superblock, BCHFS_MAGIC is 
the
new one. BCACHEFS_STATFS_MAGIC is for statfs.
Kent,
Would you please export needed magic values to 
include/uapi/linux/magic.h?
Since you are the maintainer also I'm confused about variables to 
be named.

--
Su

>  enum tst_fill_access_pattern {
>  	TST_FILL_BLOCKS,
>  	TST_FILL_RANDOM
> diff --git a/lib/tst_fs_type.c b/lib/tst_fs_type.c
> index d9c9c0817..7f0d61a8e 100644
> --- a/lib/tst_fs_type.c
> +++ b/lib/tst_fs_type.c
> @@ -36,6 +36,8 @@ const char *tst_fs_type_name(long f_type)
>  		return "9p";
>  	case TST_RAMFS_MAGIC:
>  		return "ramfs";
> +	case TST_BCACHE_MAGIC:
> +		return "bcachefs";
>  	case TST_BTRFS_MAGIC:
>  		return "btrfs";
>  	case TST_XFS_MAGIC:

Re: [PATCH v2 1/3] lib: Add Bcachefs magic

[Petr Vorel]

Hi all,


> On Thu 07 Dec 2023 at 20:40, Petr Vorel <pvorel@suse.cz> wrote:

> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > New in v2

> >  include/tst_fs.h  | 3 +++
> >  lib/tst_fs_type.c | 2 ++
> >  2 files changed, 5 insertions(+)

> > diff --git a/include/tst_fs.h b/include/tst_fs.h
> > index eb9841165..06f5be896 100644
> > --- a/include/tst_fs.h
> > +++ b/include/tst_fs.h
> > @@ -34,6 +34,9 @@
> >  #define TST_VFAT_MAGIC     0x4d44 /* AKA MSDOS */
> >  #define TST_EXFAT_MAGIC    0x2011BAB0UL

> > +/* fs/bcachefs/bcachefs_format.h */
> > +#define TST_BCACHE_MAGIC		0xca451a4e
> > +

> Not object to the patch. There are three MAGIC values in
> bcachefs_format.h: BCACHE_MAGIC, BCHFS_MAGIC, BCACHEFS_STATFS_MAGIC.

> IIUC, BCACHE_MAGIC is the older one in superblock, BCHFS_MAGIC is the
> new one. BCACHEFS_STATFS_MAGIC is for statfs.
> Kent,
> Would you please export needed magic values to include/uapi/linux/magic.h?

+1, very good idea.

Kind regards,
Petr

> Since you are the maintainer also I'm confused about variables to be named.

[PATCH v2 2/3] lib: Add support bcachefs filesystem to .all_filesystems

[Petr Vorel]

bcachefs has been merged into v6.7-rc1 [1]. Let's add it's support to
LTP .all_filesystems = 1 and TST_ALL_FILESYSTEMS=1.

[1] https://lwn.net/Articles/934692/

Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
The same as in v1

 lib/tst_supported_fs_types.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c
index d4911fa3b..369836717 100644
--- a/lib/tst_supported_fs_types.c
+++ b/lib/tst_supported_fs_types.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2017 Cyril Hrubis <chrubis@suse.cz>
+ * Copyright (c) Linux Test Project, 2018-2023
  */
 
 #include <stdio.h>
@@ -24,6 +25,7 @@ static const char *const fs_type_whitelist[] = {
 	"ext4",
 	"xfs",
 	"btrfs",
+	"bcachefs",
 	"vfat",
 	"exfat",
 	"ntfs",
-- 
2.43.0

[PATCH v2 3/3] statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

[Petr Vorel]

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(-)

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 */
-- 
2.43.0

Re: [PATCH v2 3/3] statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

[Kent Overstreet]

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?

> 
> 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(-)
> 
> 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 */
> -- 
> 2.43.0
> 

Re: [PATCH v2 3/3] statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

[Cyril Hrubis]

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.

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [PATCH v2 3/3] statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

[Kent Overstreet]

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?

Re: [PATCH v2 3/3] statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

[Cyril Hrubis]

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.

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [LTP] [PATCH v2 3/3] statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

[Cyril Hrubis]

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?

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [LTP] [PATCH v2 3/3] statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

[Petr Vorel]

> 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?

Kent, gentle ping ^.

Kind regards,
Petr

Re: [LTP] [PATCH v2 3/3] statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

[Kent Overstreet]

On Tue, May 07, 2024 at 10:18:46AM +0200, Petr Vorel wrote:
> > 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?
> 
> Kent, gentle ping ^.

Add it to the skiplist for now, thanks

Re: [LTP] [PATCH v2 3/3] statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs

[Petr Vorel]

> On Tue, May 07, 2024 at 10:18:46AM +0200, Petr Vorel wrote:
> > > 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?

> > Kent, gentle ping ^.

> Add it to the skiplist for now, thanks

Thanks, whitelist merged with your ABT.

Kind regards,
Petr

Re: [PATCH v2 0/3] Add support bcachefs filesystem

[Jan Kara]

On Thu 07-12-23 20:40:08, Petr Vorel wrote:
> Hi all,
> 
> @Amir @Jan, could you please have look on fanotify failures on Bcachefs?
> fanotify13.c, fanotify15.c and fanotify16.c produce many errors.
> 
> To test, just apply this patchset and then run with LTP_SINGLE_FS_TYPE:
> 
> # LTP_SINGLE_FS_TYPE=bcachefs ./fanotify15

I suspect some catch with fsids but we'll see... I hopefully get to this
tomorrow.

								Honza

> changes v1->v2:
> Two new commits:
> * lib: Add Bcachefs magic
> * statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs
> 
> Petr Vorel (3):
>   lib: Add Bcachefs magic
>   lib: Add support bcachefs filesystem to .all_filesystems
>   statx04: Skip STATX_ATTR_COMPRESSED on Bcachefs
> 
>  include/tst_fs.h                          | 3 +++
>  lib/tst_fs_type.c                         | 2 ++
>  lib/tst_supported_fs_types.c              | 2 ++
>  testcases/kernel/syscalls/statx/statx04.c | 5 +++--
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2 0/3] Add support bcachefs filesystem

[Jan Kara]

On Tue 12-12-23 15:48:21, Jan Kara wrote:
> On Thu 07-12-23 20:40:08, Petr Vorel wrote:
> > Hi all,
> > 
> > @Amir @Jan, could you please have look on fanotify failures on Bcachefs?
> > fanotify13.c, fanotify15.c and fanotify16.c produce many errors.
> > 
> > To test, just apply this patchset and then run with LTP_SINGLE_FS_TYPE:
> > 
> > # LTP_SINGLE_FS_TYPE=bcachefs ./fanotify15
> 
> I suspect some catch with fsids but we'll see... I hopefully get to this
> tomorrow.

Ok, this was a bug in bch_encode_fh(). Fix submitted.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2 0/3] Add support bcachefs filesystem

[Cyril Hrubis]

Hi!
I suppose that it's fine to merge the first two patches to enable
bcachefs in LTP before there release.

For the first two patches:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>


For the last patch that disables check for STATX_ATTR_COMPRESSED it
looks like this is missing plumbing in bcachefs so I would avoid adding
workarounds into LTP for now.

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [PATCH v2 0/3] Add support bcachefs filesystem

[Petr Vorel]

Hi Cyril,

> Hi!
> I suppose that it's fine to merge the first two patches to enable
> bcachefs in LTP before there release.

> For the first two patches:

> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

Thanks a lot, merged these two. We now have bcachefs support in LTP :).


> For the last patch that disables check for STATX_ATTR_COMPRESSED it
> looks like this is missing plumbing in bcachefs so I would avoid adding
> workarounds into LTP for now.

Sure, it makes sense. Hopefully Kent finds time to fix it soon.

Kind regards,
Petr

ENODEV on bcachefs on kernel < 6.7 [was: Re: [PATCH v2 0/3] Add support bcachefs filesystem]

[pvorel]

Hi all,

I'm sorry I haven't tested LTP with bcachefs-tools and kernel < 6.7. 
Quite a lot of tests fails:

# LTP_SINGLE_FS_TYPE=bcachefs ./chdir01
tst_buffers.c:56: TINFO: Test is using guarded buffers
tst_device.c:96: TINFO: Found free device 0 '/dev/loop0'
tst_test.c:1709: TINFO: LTP version: 20230929-269-g958402a95
tst_test.c:1593: TINFO: Timeout per run is 0h 00m 30s
tst_supported_fs_types.c:151: TINFO: WARNING: testing only bcachefs
tst_supported_fs_types.c:122: TINFO: FUSE does support bcachefs
tst_supported_fs_types.c:57: TINFO: mkfs.bcachefs does exist
tst_test.c:1669: TINFO: === Testing on bcachefs ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with bcachefs opts='' 
extra opts=''
tst_test.c:1131: TINFO: Mounting /dev/loop0 to 
/tmp/LTP_chdwqhGtZ/mntpoint fstyp=bcachefs flags=0
tst_test.c:1131: TBROK: mount(/dev/loop0, mntpoint, bcachefs, 0, (nil)) 
failed: ENODEV (19)

Library fails on safe_mount(). I wonder if we should check for kernel >= 
6.7 in case of bcachefs, or if we should return 0 in safe_mount() in 
case of bcachefs and errno == ENODEV (and possibly kernel < 6.7).

Kind regards,
Petr