[PATCHSET] xfsprogs: random bug fixes

[PATCHSET] xfsprogs: random bug fixes

[Darrick J. Wong]

Hi all,

Here's a pile of assorted bug fixes from around the codebase.

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

With a bit of luck, this should all go splendidly.
Comments and questions are, as always, welcome.

The only unreviewed patches are these:
  [PATCH 3/5] xfs_io: redefine what statx -m all does
  [PATCH 4/5] xfs_io: make statx mask parsing more generally useful

(I haven't seen a for-next push, so that's why I'm resending the other
accumulated fixes.  They haven't changed much.)

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes-6.15
---
Commits in this patchset:
 * man: fix missing cachestat manpage
 * xfs_io: catch statx fields up to 6.15
 * xfs_io: redefine what statx -m all does
 * xfs_io: make statx mask parsing more generally useful
 * mkfs: fix blkid probe API violations causing weird output
---
 io/statx.h            |   33 ++++++++++++
 io/stat.c             |  130 +++++++++++++++++++++++++++++++++++++++++++------
 libxfs/topology.c     |    3 +
 m4/package_libcdev.m4 |    2 -
 man/man8/xfs_io.8     |   17 ++++++
 5 files changed, 163 insertions(+), 22 deletions(-)

[PATCH 1/5] man: fix missing cachestat manpage

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Fix missing cachestat documentation so that xfs/293 doesn't fail.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 man/man8/xfs_io.8 |    3 +++
 1 file changed, 3 insertions(+)


diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index df508054b01cae..726e25af272242 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1101,6 +1101,9 @@ .SH FILE I/O COMMANDS
 Do not print timing information at all.
 .PD
 .RE
+.TP
+.BI "cachestat off len
+Print page cache statistics for the given file range.
 
 .SH MEMORY MAPPED I/O COMMANDS
 .TP

[PATCH 2/5] xfs_io: catch statx fields up to 6.15

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Add all the new statx fields and flags that have accumulated for the
past couple of years so they all print now.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 io/statx.h            |   25 ++++++++++++++++++++++++-
 io/stat.c             |    5 +++++
 m4/package_libcdev.m4 |    2 +-
 3 files changed, 30 insertions(+), 2 deletions(-)


diff --git a/io/statx.h b/io/statx.h
index 347f6d08210f83..273644f53cf1c4 100644
--- a/io/statx.h
+++ b/io/statx.h
@@ -138,7 +138,10 @@ struct statx {
 	__u32	stx_atomic_write_unit_max;	/* Max atomic write unit in bytes */
 	/* 0xb0 */
 	__u32   stx_atomic_write_segments_max;	/* Max atomic write segment count */
-	__u32   __spare1[1];
+
+	/* File offset alignment for direct I/O reads */
+	__u32	stx_dio_read_offset_align;
+
 	/* 0xb8 */
 	__u64	__spare3[9];	/* Spare space for future expansion */
 	/* 0x100 */
@@ -191,8 +194,28 @@ struct statx {
 
 #endif /* STATX_TYPE */
 
+#ifndef STATX_MNT_ID
+#define STATX_MNT_ID		0x00001000U	/* Got stx_mnt_id */
+#endif
+
+#ifndef STATX_DIOALIGN
+#define STATX_DIOALIGN		0x00002000U	/* Want/got direct I/O alignment info */
+#endif
+
+#ifndef STATX_MNT_ID_UNIQUE
+#define STATX_MNT_ID_UNIQUE	0x00004000U	/* Want/got extended stx_mount_id */
+#endif
+
+#ifndef STATX_SUBVOL
+#define STATX_SUBVOL		0x00008000U	/* Want/got stx_subvol */
+#endif
+
 #ifndef STATX_WRITE_ATOMIC
 #define STATX_WRITE_ATOMIC	0x00010000U	/* Want/got atomic_write_* fields */
 #endif
 
+#ifndef STATX_DIO_READ_ALIGN
+#define STATX_DIO_READ_ALIGN	0x00020000U	/* Want/got dio read alignment info */
+#endif
+
 #endif /* XFS_IO_STATX_H */
diff --git a/io/stat.c b/io/stat.c
index d27f916800c00a..b37b1a12b8b2fd 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -365,9 +365,14 @@ dump_raw_statx(struct statx *stx)
 	printf("stat.rdev_minor = %u\n", stx->stx_rdev_minor);
 	printf("stat.dev_major = %u\n", stx->stx_dev_major);
 	printf("stat.dev_minor = %u\n", stx->stx_dev_minor);
+	printf("stat.mnt_id = 0x%llu\n", (unsigned long long)stx->stx_mnt_id);
+	printf("stat.dio_mem_align = %u\n", stx->stx_dio_mem_align);
+	printf("stat.dio_offset_align = %u\n", stx->stx_dio_offset_align);
+	printf("stat.subvol = 0x%llu\n", (unsigned long long)stx->stx_subvol);
 	printf("stat.atomic_write_unit_min = %u\n", stx->stx_atomic_write_unit_min);
 	printf("stat.atomic_write_unit_max = %u\n", stx->stx_atomic_write_unit_max);
 	printf("stat.atomic_write_segments_max = %u\n", stx->stx_atomic_write_segments_max);
+	printf("stat.dio_read_offset_align = %u\n", stx->stx_dio_read_offset_align);
 	return 0;
 }
 
diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
index af9da8124dbdc8..61353d0aa9d536 100644
--- a/m4/package_libcdev.m4
+++ b/m4/package_libcdev.m4
@@ -126,7 +126,7 @@ AC_DEFUN([AC_NEED_INTERNAL_FSCRYPT_POLICY_V2],
 AC_DEFUN([AC_NEED_INTERNAL_STATX],
   [ AC_CHECK_TYPE(struct statx,
       [
-        AC_CHECK_MEMBER(struct statx.stx_atomic_write_unit_min,
+        AC_CHECK_MEMBER(struct statx.stx_dio_read_offset_align,
           ,
           need_internal_statx=yes,
           [#include <linux/stat.h>]

[PATCH 3/5] xfs_io: redefine what statx -m all does

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

As of kernel commit 581701b7efd60b ("uapi: deprecate STATX_ALL"),
STATX_ALL is deprecated and has been withdrawn from the kernel codebase.
The symbol still exists for userspace to avoid compilation breakage, but
we're all suppose to stop using it.

Therefore, redefine statx -m all to set all the bits except for the
reserved bit since it's pretty silly that "all" doesn't actually get you
all the fields.

Update the STATX_ALL definition in io/statx.h so people stop using it.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 io/statx.h        |    8 +++++++-
 io/stat.c         |    7 ++++---
 man/man8/xfs_io.8 |    3 ++-
 3 files changed, 13 insertions(+), 5 deletions(-)


diff --git a/io/statx.h b/io/statx.h
index 273644f53cf1c4..f7ef1d2784a2a9 100644
--- a/io/statx.h
+++ b/io/statx.h
@@ -170,9 +170,15 @@ struct statx {
 #define STATX_BLOCKS		0x00000400U	/* Want/got stx_blocks */
 #define STATX_BASIC_STATS	0x000007ffU	/* The stuff in the normal stat struct */
 #define STATX_BTIME		0x00000800U	/* Want/got stx_btime */
-#define STATX_ALL		0x00000fffU	/* All currently supported flags */
 #define STATX__RESERVED		0x80000000U	/* Reserved for future struct statx expansion */
 
+/*
+ * This is deprecated, and shall remain the same value in the future.  To avoid
+ * confusion please use the equivalent (STATX_BASIC_STATS | STATX_BTIME)
+ * instead.
+ */
+#define STATX_ALL		0x00000fffU
+
 /*
  * Attributes to be found in stx_attributes
  *
diff --git a/io/stat.c b/io/stat.c
index b37b1a12b8b2fd..52e2d33010a99a 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -332,7 +332,8 @@ statx_help(void)
 " -v -- More verbose output\n"
 " -r -- Print raw statx structure fields\n"
 " -m mask -- Specify the field mask for the statx call\n"
-"            (can also be 'basic' or 'all'; default STATX_ALL)\n"
+"            (can also be 'basic' or 'all'; defaults to\n"
+"             STATX_BASIC_STATS | STATX_BTIME)\n"
 " -D -- Don't sync attributes with the server\n"
 " -F -- Force the attributes to be sync'd with the server\n"
 "\n"));
@@ -391,7 +392,7 @@ statx_f(
 	char		*p;
 	struct statx	stx;
 	int		atflag = 0;
-	unsigned int	mask = STATX_ALL;
+	unsigned int	mask = STATX_BASIC_STATS | STATX_BTIME;
 
 	while ((c = getopt(argc, argv, "m:rvFD")) != EOF) {
 		switch (c) {
@@ -399,7 +400,7 @@ statx_f(
 			if (strcmp(optarg, "basic") == 0)
 				mask = STATX_BASIC_STATS;
 			else if (strcmp(optarg, "all") == 0)
-				mask = STATX_ALL;
+				mask = ~STATX__RESERVED;
 			else {
 				mask = strtoul(optarg, &p, 0);
 				if (!p || p == optarg) {
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 726e25af272242..198215103812c6 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -994,7 +994,8 @@ .SH FILE I/O COMMANDS
 Set the field mask for the statx call to STATX_BASIC_STATS.
 .TP
 .B \-m all
-Set the the field mask for the statx call to STATX_ALL (default).
+Set all bits in the field mask for the statx call except for STATX__RESERVED.
+The default is to set STATX_BASIC_STATS and STATX_BTIME.
 .TP
 .B \-m <mask>
 Specify a numeric field mask for the statx call.

[PATCH 4/5] xfs_io: make statx mask parsing more generally useful

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Enhance the statx -m parsing to be more useful:

Add words for all the new STATX_* field flags added in the previous
patch.

Allow "+" and "-" prefixes to add or remove flags from the mask.

Allow multiple arguments to be specified as a comma separated list.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 io/stat.c         |  120 ++++++++++++++++++++++++++++++++++++++++++++++-------
 man/man8/xfs_io.8 |   11 +++++
 2 files changed, 116 insertions(+), 15 deletions(-)


diff --git a/io/stat.c b/io/stat.c
index 52e2d33010a99a..c3a4bb15229ee5 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -321,10 +321,41 @@ _statx(
 #endif
 }
 
+struct statx_masks {
+	const char	*name;
+	unsigned int	mask;
+};
+
+static const struct statx_masks statx_masks[] = {
+	{"basic",		STATX_BASIC_STATS},
+	{"all",			~STATX__RESERVED},
+
+	{"type",		STATX_TYPE},
+	{"mode",		STATX_MODE},
+	{"nlink",		STATX_NLINK},
+	{"uid",			STATX_UID},
+	{"gid",			STATX_GID},
+	{"atime",		STATX_ATIME},
+	{"mtime",		STATX_MTIME},
+	{"ctime",		STATX_CTIME},
+	{"ino",			STATX_INO},
+	{"size",		STATX_SIZE},
+	{"blocks",		STATX_BLOCKS},
+	{"btime",		STATX_BTIME},
+	{"mnt_id",		STATX_MNT_ID},
+	{"dioalign",		STATX_DIOALIGN},
+	{"mnt_id_unique",	STATX_MNT_ID_UNIQUE},
+	{"subvol",		STATX_SUBVOL},
+	{"write_atomic",	STATX_WRITE_ATOMIC},
+	{"dio_read_align",	STATX_DIO_READ_ALIGN},
+};
+
 static void
 statx_help(void)
 {
-        printf(_(
+	unsigned int	i;
+
+	printf(_(
 "\n"
 " Display extended file status.\n"
 "\n"
@@ -334,9 +365,16 @@ statx_help(void)
 " -m mask -- Specify the field mask for the statx call\n"
 "            (can also be 'basic' or 'all'; defaults to\n"
 "             STATX_BASIC_STATS | STATX_BTIME)\n"
+" -m +mask -- Add this to the field mask for the statx call\n"
+" -m -mask -- Remove this from the field mask for the statx call\n"
 " -D -- Don't sync attributes with the server\n"
 " -F -- Force the attributes to be sync'd with the server\n"
-"\n"));
+"\n"
+"statx mask values: "));
+
+	for (i = 0; i < ARRAY_SIZE(statx_masks); i++)
+		printf("%s%s", i == 0 ? "" : ", ", statx_masks[i].name);
+	printf("\n");
 }
 
 /* statx helper */
@@ -377,6 +415,68 @@ dump_raw_statx(struct statx *stx)
 	return 0;
 }
 
+enum statx_mask_op {
+	SET,
+	REMOVE,
+	ADD,
+};
+
+static bool
+parse_statx_masks(
+	char			*optarg,
+	unsigned int		*caller_mask)
+{
+	char			*arg = optarg;
+	char			*word;
+	unsigned int		i;
+
+	while ((word = strtok(arg, ",")) != NULL) {
+		enum statx_mask_op op;
+		unsigned int	mask;
+		char		*p;
+
+		arg = NULL;
+
+		if (*word == '+') {
+			op = ADD;
+			word++;
+		} else if (*word == '-') {
+			op = REMOVE;
+			word++;
+		} else {
+			op = SET;
+		}
+
+		for (i = 0; i < ARRAY_SIZE(statx_masks); i++) {
+			if (!strcmp(statx_masks[i].name, word)) {
+				mask = statx_masks[i].mask;
+				goto process_op;
+			}
+		}
+
+		mask = strtoul(word, &p, 0);
+		if (!p || p == word) {
+			printf( _("non-numeric mask -- %s\n"), word);
+			return false;
+		}
+
+process_op:
+		switch (op) {
+		case ADD:
+			*caller_mask |= mask;
+			continue;
+		case REMOVE:
+			*caller_mask &= ~mask;
+			continue;
+		case SET:
+			*caller_mask = mask;
+			continue;
+		}
+	}
+
+	return true;
+}
+
 /*
  * options:
  * 	- input flags - query type
@@ -389,7 +489,6 @@ statx_f(
 	char		**argv)
 {
 	int		c, verbose = 0, raw = 0;
-	char		*p;
 	struct statx	stx;
 	int		atflag = 0;
 	unsigned int	mask = STATX_BASIC_STATS | STATX_BTIME;
@@ -397,18 +496,9 @@ statx_f(
 	while ((c = getopt(argc, argv, "m:rvFD")) != EOF) {
 		switch (c) {
 		case 'm':
-			if (strcmp(optarg, "basic") == 0)
-				mask = STATX_BASIC_STATS;
-			else if (strcmp(optarg, "all") == 0)
-				mask = ~STATX__RESERVED;
-			else {
-				mask = strtoul(optarg, &p, 0);
-				if (!p || p == optarg) {
-					printf(
-				_("non-numeric mask -- %s\n"), optarg);
-					exitcode = 1;
-					return 0;
-				}
+			if (!parse_statx_masks(optarg, &mask)) {
+				exitcode = 1;
+				return 0;
 			}
 			break;
 		case 'r':
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 198215103812c6..64b3e907553f48 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1000,6 +1000,17 @@ .SH FILE I/O COMMANDS
 .B \-m <mask>
 Specify a numeric field mask for the statx call.
 .TP
+.BI "\-m +" value
+Add this value to the statx field value.
+Values can be numeric, or they can be words describing the desired fields.
+See the help command output for a list of recognized words.
+.TP
+.BI "\-m -" value
+Remove this value from the statx field value.
+.TP
+.BI "\-m +" value ",-" value
+Add and remove multiple values from the statx field value.
+.TP
 .B \-F
 Force the attributes to be synced with the server.
 .TP

[PATCH 5/5] mkfs: fix blkid probe API violations causing weird output

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

The blkid_do_fullprobe function in libblkid 2.38.1 will try to read the
last 512 bytes off the end of a block device.  If the block device has a
2k LBA size, that read will fail.  blkid_do_fullprobe passes the -EIO
back to the caller (mkfs) even though the API documentation says it
only returns 1, 0, or -1.

Change the "cannot detect existing fs" logic to look for any negative
number.  Otherwise, you get unhelpful output like this:

$ mkfs.xfs -l size=32m -b size=4096 /dev/loop3
mkfs.xfs: Use the -f option to force overwrite.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 libxfs/topology.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/libxfs/topology.c b/libxfs/topology.c
index 8c6affb4c4e436..96ee74b61b30f5 100644
--- a/libxfs/topology.c
+++ b/libxfs/topology.c
@@ -205,7 +205,8 @@ check_overwrite(
 out:
 	if (pr)
 		blkid_free_probe(pr);
-	if (ret == -1)
+	/* libblkid 2.38.1 lies and can return -EIO */
+	if (ret < 0)
 		fprintf(stderr,
 			_("%s: probe of %s failed, cannot detect "
 			  "existing filesystem.\n"), progname, device);

Re: [PATCH 3/5] xfs_io: redefine what statx -m all does

[Andrey Albershteyn]

On 2025-04-24 14:53:23, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> As of kernel commit 581701b7efd60b ("uapi: deprecate STATX_ALL"),
> STATX_ALL is deprecated and has been withdrawn from the kernel codebase.
> The symbol still exists for userspace to avoid compilation breakage, but
> we're all suppose to stop using it.
> 
> Therefore, redefine statx -m all to set all the bits except for the
> reserved bit since it's pretty silly that "all" doesn't actually get you
> all the fields.
> 
> Update the STATX_ALL definition in io/statx.h so people stop using it.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

lgtm
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>

-- 
- Andrey

Re: [PATCH 4/5] xfs_io: make statx mask parsing more generally useful

[Andrey Albershteyn]

On 2025-04-24 14:53:39, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Enhance the statx -m parsing to be more useful:
> 
> Add words for all the new STATX_* field flags added in the previous
> patch.
> 
> Allow "+" and "-" prefixes to add or remove flags from the mask.
> 
> Allow multiple arguments to be specified as a comma separated list.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

lgtm
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>

-- 
- Andrey

Re: [PATCH 3/5] xfs_io: redefine what statx -m all does

[Christoph Hellwig]

On Thu, Apr 24, 2025 at 02:53:23PM -0700, Darrick J. Wong wrote:
> +The default is to set STATX_BASIC_STATS and STATX_BTIME.

The default without this options is to set STATX_BASIC_STATS and STATX_BTIME.

?

Otherwise looks good:


Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/5] xfs_io: make statx mask parsing more generally useful

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/5] xfs_io: catch statx fields up to 6.15

[John Garry]

On 24/04/2025 22:53, Darrick J. Wong wrote:
> From: Darrick J. Wong<djwong@kernel.org>
> 
> Add all the new statx fields and flags that have accumulated for the
> past couple of years so they all print now.
> 
> Signed-off-by: "Darrick J. Wong"<djwong@kernel.org>
> Reviewed-by: Christoph Hellwig<hch@lst.de>

FWIW:
Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCH 3/5] xfs_io: redefine what statx -m all does

[John Garry]

On 24/04/2025 22:53, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> As of kernel commit 581701b7efd60b ("uapi: deprecate STATX_ALL"),
> STATX_ALL is deprecated and has been withdrawn from the kernel codebase.
> The symbol still exists for userspace to avoid compilation breakage, but
> we're all suppose to stop using it.
> 
> Therefore, redefine statx -m all to set all the bits except for the
> reserved bit since it's pretty silly that "all" doesn't actually get you
> all the fields.
> 
> Update the STATX_ALL definition in io/statx.h so people stop using it.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

FWIW:
Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCH 3/5] xfs_io: redefine what statx -m all does

[Darrick J. Wong]

On Fri, Apr 25, 2025 at 06:11:53AM -0700, Christoph Hellwig wrote:
> On Thu, Apr 24, 2025 at 02:53:23PM -0700, Darrick J. Wong wrote:
> > +The default is to set STATX_BASIC_STATS and STATX_BTIME.
> 
> The default without this options is to set STATX_BASIC_STATS and STATX_BTIME.

How about:
"If no -m arguments are specified, the default is to set
STATX_BASIC_STATS and STATX_BTIME."

Thanks all for reviewing this.

--D

> ?
> 
> Otherwise looks good:
> 
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/5] xfs_io: redefine what statx -m all does

[Christoph Hellwig]

On Fri, Apr 25, 2025 at 07:50:01AM -0700, Darrick J. Wong wrote:
> On Fri, Apr 25, 2025 at 06:11:53AM -0700, Christoph Hellwig wrote:
> > On Thu, Apr 24, 2025 at 02:53:23PM -0700, Darrick J. Wong wrote:
> > > +The default is to set STATX_BASIC_STATS and STATX_BTIME.
> > 
> > The default without this options is to set STATX_BASIC_STATS and STATX_BTIME.
> 
> How about:
> "If no -m arguments are specified, the default is to set
> STATX_BASIC_STATS and STATX_BTIME."

Fine with me.