[PATCHSET v2 0/1] xfs_io: small improvements to statfs

[PATCHSET v2 0/1] xfs_io: small improvements to statfs

[Darrick J. Wong]

Hi all,

Change the statfs command so that you can invoke any combination of the
three state calls.

v2: Fix broken stuff pointed out by maintainer

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=statfs-fixes
---
 io/stat.c         |  140 ++++++++++++++++++++++++++++++++++++++---------------
 man/man8/xfs_io.8 |   26 ++++++++--
 2 files changed, 123 insertions(+), 43 deletions(-)

[PATCH 1/1] xfs_io: allow callers to dump fs stats individually

[Darrick J. Wong]

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

Enable callers to decide if they want to see statfs, fscounts, or
geometry information (or any combination) from the xfs_io statfs
command.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 io/stat.c         |  140 ++++++++++++++++++++++++++++++++++++++---------------
 man/man8/xfs_io.8 |   26 ++++++++--
 2 files changed, 123 insertions(+), 43 deletions(-)


diff --git a/io/stat.c b/io/stat.c
index 49c4c27c..b57f9eef 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -171,6 +171,24 @@ stat_f(
 	return 0;
 }
 
+static void
+statfs_help(void)
+{
+        printf(_(
+"\n"
+" Display file system status.\n"
+"\n"
+" Options:\n"
+" -c -- Print fs summary count data.\n"
+" -g -- Print fs geometry data.\n"
+" -s -- Print statfs data.\n"
+"\n"));
+}
+
+#define REPORT_STATFS		(1 << 0)
+#define REPORT_GEOMETRY		(1 << 1)
+#define REPORT_FSCOUNTS		(1 << 2)
+
 static int
 statfs_f(
 	int			argc,
@@ -179,55 +197,95 @@ statfs_f(
 	struct xfs_fsop_counts	fscounts;
 	struct xfs_fsop_geom	fsgeo;
 	struct statfs		st;
+	unsigned int		flags = 0;
+	int			c;
 	int			ret;
 
+	while ((c = getopt(argc, argv, "cgs")) != EOF) {
+		switch (c) {
+		case 'c':
+			flags |= REPORT_FSCOUNTS;
+			break;
+		case 'g':
+			flags |= REPORT_GEOMETRY;
+			break;
+		case 's':
+			flags |= REPORT_STATFS;
+			break;
+		default:
+			exitcode = 1;
+			return command_usage(&statfs_cmd);
+		}
+	}
+
+	if (!flags)
+		flags = REPORT_STATFS | REPORT_GEOMETRY | REPORT_FSCOUNTS;
+
 	printf(_("fd.path = \"%s\"\n"), file->name);
-	if (platform_fstatfs(file->fd, &st) < 0) {
-		perror("fstatfs");
-		exitcode = 1;
-	} else {
-		printf(_("statfs.f_bsize = %lld\n"), (long long) st.f_bsize);
-		printf(_("statfs.f_blocks = %lld\n"), (long long) st.f_blocks);
-		printf(_("statfs.f_bavail = %lld\n"), (long long) st.f_bavail);
-		printf(_("statfs.f_files = %lld\n"), (long long) st.f_files);
-		printf(_("statfs.f_ffree = %lld\n"), (long long) st.f_ffree);
+	if (flags & REPORT_STATFS) {
+		ret = platform_fstatfs(file->fd, &st);
+		if (ret < 0) {
+			perror("fstatfs");
+			exitcode = 1;
+		} else {
+			printf(_("statfs.f_bsize = %lld\n"),
+					(long long) st.f_bsize);
+			printf(_("statfs.f_blocks = %lld\n"),
+					(long long) st.f_blocks);
+			printf(_("statfs.f_bavail = %lld\n"),
+					(long long) st.f_bavail);
+			printf(_("statfs.f_files = %lld\n"),
+					(long long) st.f_files);
+			printf(_("statfs.f_ffree = %lld\n"),
+					(long long) st.f_ffree);
 #ifdef HAVE_STATFS_FLAGS
-		printf(_("statfs.f_flags = 0x%llx\n"), (long long) st.f_flags);
+			printf(_("statfs.f_flags = 0x%llx\n"),
+					(long long) st.f_flags);
 #endif
+		}
 	}
+
 	if (file->flags & IO_FOREIGN)
 		return 0;
-	ret = -xfrog_geometry(file->fd, &fsgeo);
-	if (ret) {
-		xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
-		exitcode = 1;
-	} else {
-		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
-		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
-		printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
-		printf(_("geom.datablocks = %llu\n"),
-			(unsigned long long) fsgeo.datablocks);
-		printf(_("geom.rtblocks = %llu\n"),
-			(unsigned long long) fsgeo.rtblocks);
-		printf(_("geom.rtextents = %llu\n"),
-			(unsigned long long) fsgeo.rtextents);
-		printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
-		printf(_("geom.sunit = %u\n"), fsgeo.sunit);
-		printf(_("geom.swidth = %u\n"), fsgeo.swidth);
+
+	if (flags & REPORT_GEOMETRY) {
+		ret = -xfrog_geometry(file->fd, &fsgeo);
+		if (ret) {
+			xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
+			exitcode = 1;
+		} else {
+			printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
+			printf(_("geom.agcount = %u\n"), fsgeo.agcount);
+			printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
+			printf(_("geom.datablocks = %llu\n"),
+				(unsigned long long) fsgeo.datablocks);
+			printf(_("geom.rtblocks = %llu\n"),
+				(unsigned long long) fsgeo.rtblocks);
+			printf(_("geom.rtextents = %llu\n"),
+				(unsigned long long) fsgeo.rtextents);
+			printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
+			printf(_("geom.sunit = %u\n"), fsgeo.sunit);
+			printf(_("geom.swidth = %u\n"), fsgeo.swidth);
+		}
 	}
-	if ((xfsctl(file->name, file->fd, XFS_IOC_FSCOUNTS, &fscounts)) < 0) {
-		perror("XFS_IOC_FSCOUNTS");
-		exitcode = 1;
-	} else {
-		printf(_("counts.freedata = %llu\n"),
-			(unsigned long long) fscounts.freedata);
-		printf(_("counts.freertx = %llu\n"),
-			(unsigned long long) fscounts.freertx);
-		printf(_("counts.freeino = %llu\n"),
-			(unsigned long long) fscounts.freeino);
-		printf(_("counts.allocino = %llu\n"),
-			(unsigned long long) fscounts.allocino);
+
+	if (flags & REPORT_FSCOUNTS) {
+		ret = ioctl(file->fd, XFS_IOC_FSCOUNTS, &fscounts);
+		if (ret < 0) {
+			perror("XFS_IOC_FSCOUNTS");
+			exitcode = 1;
+		} else {
+			printf(_("counts.freedata = %llu\n"),
+				(unsigned long long) fscounts.freedata);
+			printf(_("counts.freertx = %llu\n"),
+				(unsigned long long) fscounts.freertx);
+			printf(_("counts.freeino = %llu\n"),
+				(unsigned long long) fscounts.freeino);
+			printf(_("counts.allocino = %llu\n"),
+				(unsigned long long) fscounts.allocino);
+		}
 	}
+
 	return 0;
 }
 
@@ -407,9 +465,13 @@ stat_init(void)
 
 	statfs_cmd.name = "statfs";
 	statfs_cmd.cfunc = statfs_f;
+	statfs_cmd.argmin = 0;
+	statfs_cmd.argmax = -1;
+	statfs_cmd.args = _("[-c] [-g] [-s]");
 	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
 	statfs_cmd.oneline =
 		_("statistics on the filesystem of the currently open file");
+	statfs_cmd.help = statfs_help;
 
 	add_command(&stat_cmd);
 	add_command(&statx_cmd);
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 1103dc42..e3c5d3ea 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1235,11 +1235,29 @@ down, matching XFS behavior when critical corruption is encountered.
 .PD
 .RE
 .TP
-.B statfs
-Selected statistics from
+.B statfs [ -c ] [ -g ] [ -s ]
+Report selected statistics on the filesystem where the current file resides.
+The default behavior is to enable all three reporting options:
+.RS 1.0i
+.PD 0
+.TP
+.BI \-c
+Display
+.B XFS_IOC_FSCOUNTERS
+summary counter data.
+.TP
+.BI \-g
+Display
+.B XFS_IOC_FSGEOMETRY
+filesystem geometry data.
+.TP
+.BI \-s
+Display
 .BR statfs (2)
-and the XFS_IOC_FSGEOMETRY
-system call on the filesystem where the current file resides.
+data.
+.TP
+.RE
+.PD
 .TP
 .BI "inode  [ [ -n ] " number " ] [ -v ]"
 The inode command queries physical information about an inode. With

Re: [PATCH 1/1] xfs_io: allow callers to dump fs stats individually

[Eric Sandeen]

On 7/28/21 2:16 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Enable callers to decide if they want to see statfs, fscounts, or
> geometry information (or any combination) from the xfs_io statfs
> command.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Looks functional this time :P
(and thanks for tidying up the other bits like the default behavior etc)

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Re: [PATCH 1/1] xfs_io: allow callers to dump fs stats individually

[Eric Sandeen]

On 7/8/21 5:40 PM, Darrick J. Wong wrote:
> I'll change it to:
> 
>        statfs [ -c ] [ -g ] [ -s ]
>               Report selected statistics on the filesystem  where  the
>               current file resides.  The default behavior is to enable
>               all three reporting options:
>                  -c     Display  XFS_IOC_FSCOUNTERS  summary   counter
>                         data.
>                  -g     Display XFS_IOC_FSGEOMETRY filesystem geometry
>                         data.
>                  -s     Display statfs(2) data.
> 
> If that's ok.  I got rid of -a because it's redundant.

sure, sounds good.

-Eric

Re: [PATCH 1/1] xfs_io: allow callers to dump fs stats individually

[Darrick J. Wong]

On Thu, Jul 08, 2021 at 03:58:47PM -0500, Eric Sandeen wrote:
> On 7/2/21 9:58 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Enable callers to decide if they want to see statfs, fscounts, or
> > geometry information (or any combination) from the xfs_io statfs
> > command.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  io/stat.c         |  149 +++++++++++++++++++++++++++++++++++++++--------------
> >  man/man8/xfs_io.8 |   17 ++++++
> >  2 files changed, 127 insertions(+), 39 deletions(-)
> > 
> > 
> > diff --git a/io/stat.c b/io/stat.c
> > index 49c4c27c..1993247c 100644
> > --- a/io/stat.c
> > +++ b/io/stat.c
> > @@ -171,6 +171,26 @@ stat_f(
> >  	return 0;
> >  }
> >  
> > +static void
> > +statfs_help(void)
> > +{
> > +        printf(_(
> > +"\n"
> > +" Display file system status.\n"
> > +"\n"
> > +" Options:\n"
> > +" -a -- Print statfs, geometry, and fs summary count data.\n"
> > +" -c -- Print fs summary count data.\n"
> > +" -g -- Print fs geometry data.\n"
> > +" -s -- Print statfs data.\n"
> > +"\n"));
> > +}
> > +
> > +#define REPORT_STATFS		(1 << 0)
> > +#define REPORT_GEOMETRY		(1 << 1)
> > +#define REPORT_FSCOUNTS		(1 << 2)
> > +#define REPORT_DEFAULT		(1 << 31)
> > +
> >  static int
> >  statfs_f(
> >  	int			argc,
> > @@ -179,55 +199,102 @@ statfs_f(
> >  	struct xfs_fsop_counts	fscounts;
> >  	struct xfs_fsop_geom	fsgeo;
> >  	struct statfs		st;
> > +	unsigned int		flags = REPORT_DEFAULT;
> 
> Nitpicking this only because the patch will need to be resent anyway ;)
> Why not just "flags = 0" and "if !flags flags = ALL|THREE|FLAGS;" ?

Changed.

> (OTOH I don't really care, just wasn't sure about the need for it)
> 
> > +	int			c;
> >  	int			ret;
> >  
> > +	while ((c = getopt(argc, argv, "acgs")) != EOF) {
> > +		switch (c) {
> > +		case 'a':
> > +			flags = REPORT_STATFS | REPORT_GEOMETRY |
> > +				REPORT_FSCOUNTS;
> > +			break;
> > +		case 'c':
> > +			flags &= ~REPORT_DEFAULT;
> > +			flags |= REPORT_FSCOUNTS;
> > +			break;
> > +		case 'g':
> > +			flags &= ~REPORT_GEOMETRY;
> > +			flags |= REPORT_FSCOUNTS;
> 
> this looks ... wrong. ITYM:
> 
> +		case 'g':
> +			flags &= ~REPORT_DEFAULT;
> +			flags |= REPORT_GEOMETRY;
> 
> ?
> 
> > +			break;
> > +		case 's':
> > +			flags &= ~REPORT_STATFS;
> > +			flags |= REPORT_FSCOUNTS;
> 
> similar here

URkk, wow.  Fixed.

> 
> > +			break;
> > +		default:
> > +			exitcode = 1;
> > +			return command_usage(&statfs_cmd);
> > +		}
> > +	}
> > +
> > +	if (flags & REPORT_DEFAULT)
> > +		flags = REPORT_STATFS | REPORT_GEOMETRY | REPORT_FSCOUNTS;
> > +
> >  	printf(_("fd.path = \"%s\"\n"), file->name);
> > -	if (platform_fstatfs(file->fd, &st) < 0) {
> > -		perror("fstatfs");
> > -		exitcode = 1;
> > -	} else {
> > -		printf(_("statfs.f_bsize = %lld\n"), (long long) st.f_bsize);
> > -		printf(_("statfs.f_blocks = %lld\n"), (long long) st.f_blocks);
> > -		printf(_("statfs.f_bavail = %lld\n"), (long long) st.f_bavail);
> > -		printf(_("statfs.f_files = %lld\n"), (long long) st.f_files);
> > -		printf(_("statfs.f_ffree = %lld\n"), (long long) st.f_ffree);
> > +	if (flags & REPORT_STATFS) {
> > +		ret = platform_fstatfs(file->fd, &st);
> > +		if (ret < 0) {
> > +			perror("fstatfs");
> > +			exitcode = 1;
> > +		} else {
> > +			printf(_("statfs.f_bsize = %lld\n"),
> > +					(long long) st.f_bsize);
> > +			printf(_("statfs.f_blocks = %lld\n"),
> > +					(long long) st.f_blocks);
> > +			printf(_("statfs.f_bavail = %lld\n"),
> > +					(long long) st.f_bavail);
> > +			printf(_("statfs.f_files = %lld\n"),
> > +					(long long) st.f_files);
> > +			printf(_("statfs.f_ffree = %lld\n"),
> > +					(long long) st.f_ffree);
> >  #ifdef HAVE_STATFS_FLAGS
> > -		printf(_("statfs.f_flags = 0x%llx\n"), (long long) st.f_flags);
> > +			printf(_("statfs.f_flags = 0x%llx\n"),
> > +					(long long) st.f_flags);
> >  #endif
> > +		}
> >  	}
> > +
> >  	if (file->flags & IO_FOREIGN)
> >  		return 0;
> > -	ret = -xfrog_geometry(file->fd, &fsgeo);
> > -	if (ret) {
> > -		xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
> > -		exitcode = 1;
> > -	} else {
> > -		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
> > -		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
> > -		printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
> > -		printf(_("geom.datablocks = %llu\n"),
> > -			(unsigned long long) fsgeo.datablocks);
> > -		printf(_("geom.rtblocks = %llu\n"),
> > -			(unsigned long long) fsgeo.rtblocks);
> > -		printf(_("geom.rtextents = %llu\n"),
> > -			(unsigned long long) fsgeo.rtextents);
> > -		printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
> > -		printf(_("geom.sunit = %u\n"), fsgeo.sunit);
> > -		printf(_("geom.swidth = %u\n"), fsgeo.swidth);
> > +
> > +	if (flags & REPORT_GEOMETRY) {
> > +		ret = -xfrog_geometry(file->fd, &fsgeo);
> > +		if (ret) {
> > +			xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
> > +			exitcode = 1;
> > +		} else {
> > +			printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
> > +			printf(_("geom.agcount = %u\n"), fsgeo.agcount);
> > +			printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
> > +			printf(_("geom.datablocks = %llu\n"),
> > +				(unsigned long long) fsgeo.datablocks);
> > +			printf(_("geom.rtblocks = %llu\n"),
> > +				(unsigned long long) fsgeo.rtblocks);
> > +			printf(_("geom.rtextents = %llu\n"),
> > +				(unsigned long long) fsgeo.rtextents);
> > +			printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
> > +			printf(_("geom.sunit = %u\n"), fsgeo.sunit);
> > +			printf(_("geom.swidth = %u\n"), fsgeo.swidth);
> > +		}
> >  	}
> > -	if ((xfsctl(file->name, file->fd, XFS_IOC_FSCOUNTS, &fscounts)) < 0) {
> > -		perror("XFS_IOC_FSCOUNTS");
> > -		exitcode = 1;
> > -	} else {
> > -		printf(_("counts.freedata = %llu\n"),
> > -			(unsigned long long) fscounts.freedata);
> > -		printf(_("counts.freertx = %llu\n"),
> > -			(unsigned long long) fscounts.freertx);
> > -		printf(_("counts.freeino = %llu\n"),
> > -			(unsigned long long) fscounts.freeino);
> > -		printf(_("counts.allocino = %llu\n"),
> > -			(unsigned long long) fscounts.allocino);
> > +
> > +	if (flags & REPORT_FSCOUNTS) {
> > +		ret = ioctl(file->fd, XFS_IOC_FSCOUNTS, &fscounts);
> > +		if (ret < 0) {
> > +			perror("XFS_IOC_FSCOUNTS");
> > +			exitcode = 1;
> > +		} else {
> > +			printf(_("counts.freedata = %llu\n"),
> > +				(unsigned long long) fscounts.freedata);
> > +			printf(_("counts.freertx = %llu\n"),
> > +				(unsigned long long) fscounts.freertx);
> > +			printf(_("counts.freeino = %llu\n"),
> > +				(unsigned long long) fscounts.freeino);
> > +			printf(_("counts.allocino = %llu\n"),
> > +				(unsigned long long) fscounts.allocino);
> > +		}
> >  	}
> > +
> >  	return 0;
> >  }
> >  
> > @@ -407,9 +474,13 @@ stat_init(void)
> >  
> >  	statfs_cmd.name = "statfs";
> >  	statfs_cmd.cfunc = statfs_f;
> > +	statfs_cmd.argmin = 0;
> > +	statfs_cmd.argmax = -1;
> 
> Eh, I guess you can do "stats -a -a -a -c -c -c -s -a -a -c -c -c -g -g -g ...." and it works fine, so sure, "-1" ;)
> 
> > +	statfs_cmd.args = _("[-a] [-c] [-g] [-s]");
> >  	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> >  	statfs_cmd.oneline =
> >  		_("statistics on the filesystem of the currently open file");
> > +	statfs_cmd.help = statfs_help;
> >  
> >  	add_command(&stat_cmd);
> >  	add_command(&statx_cmd);
> > diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> > index 1103dc42..32bdd866 100644
> > --- a/man/man8/xfs_io.8
> > +++ b/man/man8/xfs_io.8
> 
> Probably want to turn this into:
> 
> statfs [-a] [-c] [-g] [-s]
> 	<description, probably updated to add XFS_IOC_FSCOUNTS to the list of $STUFF>
> 
> > @@ -1240,6 +1240,23 @@ Selected statistics from
> >  .BR statfs (2)
> >  and the XFS_IOC_FSGEOMETRY
> >  system call on the filesystem where the current file resides.
> > +.RS 1.0i
> > +.PD 0
> > +.TP
> > +.BI \-a
> > +Display statfs, geometry, and fs summary counter data.
> 
> Perhaps note that this is the default if no options are specified?

I'll change it to:

       statfs [ -c ] [ -g ] [ -s ]
              Report selected statistics on the filesystem  where  the
              current file resides.  The default behavior is to enable
              all three reporting options:
                 -c     Display  XFS_IOC_FSCOUNTERS  summary   counter
                        data.
                 -g     Display XFS_IOC_FSGEOMETRY filesystem geometry
                        data.
                 -s     Display statfs(2) data.

If that's ok.  I got rid of -a because it's redundant.

--D

> 
> > +.TP
> > +.BI \-c
> > +Display fs summary counter data.
> > +.TP
> > +.BI \-g
> > +Display geometry data.
> > +.TP
> > +.BI \-s
> > +Display statfs data.
> > +.TP
> > +.RE
> > +.PD
> >  .TP
> >  .BI "inode  [ [ -n ] " number " ] [ -v ]"
> >  The inode command queries physical information about an inode. With
> > 

Re: [PATCH 1/1] xfs_io: allow callers to dump fs stats individually

[Eric Sandeen]

On 7/2/21 9:58 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Enable callers to decide if they want to see statfs, fscounts, or
> geometry information (or any combination) from the xfs_io statfs
> command.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  io/stat.c         |  149 +++++++++++++++++++++++++++++++++++++++--------------
>  man/man8/xfs_io.8 |   17 ++++++
>  2 files changed, 127 insertions(+), 39 deletions(-)
> 
> 
> diff --git a/io/stat.c b/io/stat.c
> index 49c4c27c..1993247c 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -171,6 +171,26 @@ stat_f(
>  	return 0;
>  }
>  
> +static void
> +statfs_help(void)
> +{
> +        printf(_(
> +"\n"
> +" Display file system status.\n"
> +"\n"
> +" Options:\n"
> +" -a -- Print statfs, geometry, and fs summary count data.\n"
> +" -c -- Print fs summary count data.\n"
> +" -g -- Print fs geometry data.\n"
> +" -s -- Print statfs data.\n"
> +"\n"));
> +}
> +
> +#define REPORT_STATFS		(1 << 0)
> +#define REPORT_GEOMETRY		(1 << 1)
> +#define REPORT_FSCOUNTS		(1 << 2)
> +#define REPORT_DEFAULT		(1 << 31)
> +
>  static int
>  statfs_f(
>  	int			argc,
> @@ -179,55 +199,102 @@ statfs_f(
>  	struct xfs_fsop_counts	fscounts;
>  	struct xfs_fsop_geom	fsgeo;
>  	struct statfs		st;
> +	unsigned int		flags = REPORT_DEFAULT;

Nitpicking this only because the patch will need to be resent anyway ;)
Why not just "flags = 0" and "if !flags flags = ALL|THREE|FLAGS;" ?

(OTOH I don't really care, just wasn't sure about the need for it)

> +	int			c;
>  	int			ret;
>  
> +	while ((c = getopt(argc, argv, "acgs")) != EOF) {
> +		switch (c) {
> +		case 'a':
> +			flags = REPORT_STATFS | REPORT_GEOMETRY |
> +				REPORT_FSCOUNTS;
> +			break;
> +		case 'c':
> +			flags &= ~REPORT_DEFAULT;
> +			flags |= REPORT_FSCOUNTS;
> +			break;
> +		case 'g':
> +			flags &= ~REPORT_GEOMETRY;
> +			flags |= REPORT_FSCOUNTS;

this looks ... wrong. ITYM:

+		case 'g':
+			flags &= ~REPORT_DEFAULT;
+			flags |= REPORT_GEOMETRY;

?

> +			break;
> +		case 's':
> +			flags &= ~REPORT_STATFS;
> +			flags |= REPORT_FSCOUNTS;

similar here

> +			break;
> +		default:
> +			exitcode = 1;
> +			return command_usage(&statfs_cmd);
> +		}
> +	}
> +
> +	if (flags & REPORT_DEFAULT)
> +		flags = REPORT_STATFS | REPORT_GEOMETRY | REPORT_FSCOUNTS;
> +
>  	printf(_("fd.path = \"%s\"\n"), file->name);
> -	if (platform_fstatfs(file->fd, &st) < 0) {
> -		perror("fstatfs");
> -		exitcode = 1;
> -	} else {
> -		printf(_("statfs.f_bsize = %lld\n"), (long long) st.f_bsize);
> -		printf(_("statfs.f_blocks = %lld\n"), (long long) st.f_blocks);
> -		printf(_("statfs.f_bavail = %lld\n"), (long long) st.f_bavail);
> -		printf(_("statfs.f_files = %lld\n"), (long long) st.f_files);
> -		printf(_("statfs.f_ffree = %lld\n"), (long long) st.f_ffree);
> +	if (flags & REPORT_STATFS) {
> +		ret = platform_fstatfs(file->fd, &st);
> +		if (ret < 0) {
> +			perror("fstatfs");
> +			exitcode = 1;
> +		} else {
> +			printf(_("statfs.f_bsize = %lld\n"),
> +					(long long) st.f_bsize);
> +			printf(_("statfs.f_blocks = %lld\n"),
> +					(long long) st.f_blocks);
> +			printf(_("statfs.f_bavail = %lld\n"),
> +					(long long) st.f_bavail);
> +			printf(_("statfs.f_files = %lld\n"),
> +					(long long) st.f_files);
> +			printf(_("statfs.f_ffree = %lld\n"),
> +					(long long) st.f_ffree);
>  #ifdef HAVE_STATFS_FLAGS
> -		printf(_("statfs.f_flags = 0x%llx\n"), (long long) st.f_flags);
> +			printf(_("statfs.f_flags = 0x%llx\n"),
> +					(long long) st.f_flags);
>  #endif
> +		}
>  	}
> +
>  	if (file->flags & IO_FOREIGN)
>  		return 0;
> -	ret = -xfrog_geometry(file->fd, &fsgeo);
> -	if (ret) {
> -		xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
> -		exitcode = 1;
> -	} else {
> -		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
> -		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
> -		printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
> -		printf(_("geom.datablocks = %llu\n"),
> -			(unsigned long long) fsgeo.datablocks);
> -		printf(_("geom.rtblocks = %llu\n"),
> -			(unsigned long long) fsgeo.rtblocks);
> -		printf(_("geom.rtextents = %llu\n"),
> -			(unsigned long long) fsgeo.rtextents);
> -		printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
> -		printf(_("geom.sunit = %u\n"), fsgeo.sunit);
> -		printf(_("geom.swidth = %u\n"), fsgeo.swidth);
> +
> +	if (flags & REPORT_GEOMETRY) {
> +		ret = -xfrog_geometry(file->fd, &fsgeo);
> +		if (ret) {
> +			xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
> +			exitcode = 1;
> +		} else {
> +			printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
> +			printf(_("geom.agcount = %u\n"), fsgeo.agcount);
> +			printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
> +			printf(_("geom.datablocks = %llu\n"),
> +				(unsigned long long) fsgeo.datablocks);
> +			printf(_("geom.rtblocks = %llu\n"),
> +				(unsigned long long) fsgeo.rtblocks);
> +			printf(_("geom.rtextents = %llu\n"),
> +				(unsigned long long) fsgeo.rtextents);
> +			printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
> +			printf(_("geom.sunit = %u\n"), fsgeo.sunit);
> +			printf(_("geom.swidth = %u\n"), fsgeo.swidth);
> +		}
>  	}
> -	if ((xfsctl(file->name, file->fd, XFS_IOC_FSCOUNTS, &fscounts)) < 0) {
> -		perror("XFS_IOC_FSCOUNTS");
> -		exitcode = 1;
> -	} else {
> -		printf(_("counts.freedata = %llu\n"),
> -			(unsigned long long) fscounts.freedata);
> -		printf(_("counts.freertx = %llu\n"),
> -			(unsigned long long) fscounts.freertx);
> -		printf(_("counts.freeino = %llu\n"),
> -			(unsigned long long) fscounts.freeino);
> -		printf(_("counts.allocino = %llu\n"),
> -			(unsigned long long) fscounts.allocino);
> +
> +	if (flags & REPORT_FSCOUNTS) {
> +		ret = ioctl(file->fd, XFS_IOC_FSCOUNTS, &fscounts);
> +		if (ret < 0) {
> +			perror("XFS_IOC_FSCOUNTS");
> +			exitcode = 1;
> +		} else {
> +			printf(_("counts.freedata = %llu\n"),
> +				(unsigned long long) fscounts.freedata);
> +			printf(_("counts.freertx = %llu\n"),
> +				(unsigned long long) fscounts.freertx);
> +			printf(_("counts.freeino = %llu\n"),
> +				(unsigned long long) fscounts.freeino);
> +			printf(_("counts.allocino = %llu\n"),
> +				(unsigned long long) fscounts.allocino);
> +		}
>  	}
> +
>  	return 0;
>  }
>  
> @@ -407,9 +474,13 @@ stat_init(void)
>  
>  	statfs_cmd.name = "statfs";
>  	statfs_cmd.cfunc = statfs_f;
> +	statfs_cmd.argmin = 0;
> +	statfs_cmd.argmax = -1;

Eh, I guess you can do "stats -a -a -a -c -c -c -s -a -a -c -c -c -g -g -g ...." and it works fine, so sure, "-1" ;)

> +	statfs_cmd.args = _("[-a] [-c] [-g] [-s]");
>  	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
>  	statfs_cmd.oneline =
>  		_("statistics on the filesystem of the currently open file");
> +	statfs_cmd.help = statfs_help;
>  
>  	add_command(&stat_cmd);
>  	add_command(&statx_cmd);
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 1103dc42..32bdd866 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8

Probably want to turn this into:

statfs [-a] [-c] [-g] [-s]
	<description, probably updated to add XFS_IOC_FSCOUNTS to the list of $STUFF>

> @@ -1240,6 +1240,23 @@ Selected statistics from
>  .BR statfs (2)
>  and the XFS_IOC_FSGEOMETRY
>  system call on the filesystem where the current file resides.
> +.RS 1.0i
> +.PD 0
> +.TP
> +.BI \-a
> +Display statfs, geometry, and fs summary counter data.

Perhaps note that this is the default if no options are specified?

> +.TP
> +.BI \-c
> +Display fs summary counter data.
> +.TP
> +.BI \-g
> +Display geometry data.
> +.TP
> +.BI \-s
> +Display statfs data.
> +.TP
> +.RE
> +.PD
>  .TP
>  .BI "inode  [ [ -n ] " number " ] [ -v ]"
>  The inode command queries physical information about an inode. With
> 

[PATCH 1/1] xfs_io: allow callers to dump fs stats individually

[Darrick J. Wong]

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

Enable callers to decide if they want to see statfs, fscounts, or
geometry information (or any combination) from the xfs_io statfs
command.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 io/stat.c         |  149 +++++++++++++++++++++++++++++++++++++++--------------
 man/man8/xfs_io.8 |   17 ++++++
 2 files changed, 127 insertions(+), 39 deletions(-)


diff --git a/io/stat.c b/io/stat.c
index 49c4c27c..1993247c 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -171,6 +171,26 @@ stat_f(
 	return 0;
 }
 
+static void
+statfs_help(void)
+{
+        printf(_(
+"\n"
+" Display file system status.\n"
+"\n"
+" Options:\n"
+" -a -- Print statfs, geometry, and fs summary count data.\n"
+" -c -- Print fs summary count data.\n"
+" -g -- Print fs geometry data.\n"
+" -s -- Print statfs data.\n"
+"\n"));
+}
+
+#define REPORT_STATFS		(1 << 0)
+#define REPORT_GEOMETRY		(1 << 1)
+#define REPORT_FSCOUNTS		(1 << 2)
+#define REPORT_DEFAULT		(1 << 31)
+
 static int
 statfs_f(
 	int			argc,
@@ -179,55 +199,102 @@ statfs_f(
 	struct xfs_fsop_counts	fscounts;
 	struct xfs_fsop_geom	fsgeo;
 	struct statfs		st;
+	unsigned int		flags = REPORT_DEFAULT;
+	int			c;
 	int			ret;
 
+	while ((c = getopt(argc, argv, "acgs")) != EOF) {
+		switch (c) {
+		case 'a':
+			flags = REPORT_STATFS | REPORT_GEOMETRY |
+				REPORT_FSCOUNTS;
+			break;
+		case 'c':
+			flags &= ~REPORT_DEFAULT;
+			flags |= REPORT_FSCOUNTS;
+			break;
+		case 'g':
+			flags &= ~REPORT_GEOMETRY;
+			flags |= REPORT_FSCOUNTS;
+			break;
+		case 's':
+			flags &= ~REPORT_STATFS;
+			flags |= REPORT_FSCOUNTS;
+			break;
+		default:
+			exitcode = 1;
+			return command_usage(&statfs_cmd);
+		}
+	}
+
+	if (flags & REPORT_DEFAULT)
+		flags = REPORT_STATFS | REPORT_GEOMETRY | REPORT_FSCOUNTS;
+
 	printf(_("fd.path = \"%s\"\n"), file->name);
-	if (platform_fstatfs(file->fd, &st) < 0) {
-		perror("fstatfs");
-		exitcode = 1;
-	} else {
-		printf(_("statfs.f_bsize = %lld\n"), (long long) st.f_bsize);
-		printf(_("statfs.f_blocks = %lld\n"), (long long) st.f_blocks);
-		printf(_("statfs.f_bavail = %lld\n"), (long long) st.f_bavail);
-		printf(_("statfs.f_files = %lld\n"), (long long) st.f_files);
-		printf(_("statfs.f_ffree = %lld\n"), (long long) st.f_ffree);
+	if (flags & REPORT_STATFS) {
+		ret = platform_fstatfs(file->fd, &st);
+		if (ret < 0) {
+			perror("fstatfs");
+			exitcode = 1;
+		} else {
+			printf(_("statfs.f_bsize = %lld\n"),
+					(long long) st.f_bsize);
+			printf(_("statfs.f_blocks = %lld\n"),
+					(long long) st.f_blocks);
+			printf(_("statfs.f_bavail = %lld\n"),
+					(long long) st.f_bavail);
+			printf(_("statfs.f_files = %lld\n"),
+					(long long) st.f_files);
+			printf(_("statfs.f_ffree = %lld\n"),
+					(long long) st.f_ffree);
 #ifdef HAVE_STATFS_FLAGS
-		printf(_("statfs.f_flags = 0x%llx\n"), (long long) st.f_flags);
+			printf(_("statfs.f_flags = 0x%llx\n"),
+					(long long) st.f_flags);
 #endif
+		}
 	}
+
 	if (file->flags & IO_FOREIGN)
 		return 0;
-	ret = -xfrog_geometry(file->fd, &fsgeo);
-	if (ret) {
-		xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
-		exitcode = 1;
-	} else {
-		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
-		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
-		printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
-		printf(_("geom.datablocks = %llu\n"),
-			(unsigned long long) fsgeo.datablocks);
-		printf(_("geom.rtblocks = %llu\n"),
-			(unsigned long long) fsgeo.rtblocks);
-		printf(_("geom.rtextents = %llu\n"),
-			(unsigned long long) fsgeo.rtextents);
-		printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
-		printf(_("geom.sunit = %u\n"), fsgeo.sunit);
-		printf(_("geom.swidth = %u\n"), fsgeo.swidth);
+
+	if (flags & REPORT_GEOMETRY) {
+		ret = -xfrog_geometry(file->fd, &fsgeo);
+		if (ret) {
+			xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
+			exitcode = 1;
+		} else {
+			printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
+			printf(_("geom.agcount = %u\n"), fsgeo.agcount);
+			printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
+			printf(_("geom.datablocks = %llu\n"),
+				(unsigned long long) fsgeo.datablocks);
+			printf(_("geom.rtblocks = %llu\n"),
+				(unsigned long long) fsgeo.rtblocks);
+			printf(_("geom.rtextents = %llu\n"),
+				(unsigned long long) fsgeo.rtextents);
+			printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
+			printf(_("geom.sunit = %u\n"), fsgeo.sunit);
+			printf(_("geom.swidth = %u\n"), fsgeo.swidth);
+		}
 	}
-	if ((xfsctl(file->name, file->fd, XFS_IOC_FSCOUNTS, &fscounts)) < 0) {
-		perror("XFS_IOC_FSCOUNTS");
-		exitcode = 1;
-	} else {
-		printf(_("counts.freedata = %llu\n"),
-			(unsigned long long) fscounts.freedata);
-		printf(_("counts.freertx = %llu\n"),
-			(unsigned long long) fscounts.freertx);
-		printf(_("counts.freeino = %llu\n"),
-			(unsigned long long) fscounts.freeino);
-		printf(_("counts.allocino = %llu\n"),
-			(unsigned long long) fscounts.allocino);
+
+	if (flags & REPORT_FSCOUNTS) {
+		ret = ioctl(file->fd, XFS_IOC_FSCOUNTS, &fscounts);
+		if (ret < 0) {
+			perror("XFS_IOC_FSCOUNTS");
+			exitcode = 1;
+		} else {
+			printf(_("counts.freedata = %llu\n"),
+				(unsigned long long) fscounts.freedata);
+			printf(_("counts.freertx = %llu\n"),
+				(unsigned long long) fscounts.freertx);
+			printf(_("counts.freeino = %llu\n"),
+				(unsigned long long) fscounts.freeino);
+			printf(_("counts.allocino = %llu\n"),
+				(unsigned long long) fscounts.allocino);
+		}
 	}
+
 	return 0;
 }
 
@@ -407,9 +474,13 @@ stat_init(void)
 
 	statfs_cmd.name = "statfs";
 	statfs_cmd.cfunc = statfs_f;
+	statfs_cmd.argmin = 0;
+	statfs_cmd.argmax = -1;
+	statfs_cmd.args = _("[-a] [-c] [-g] [-s]");
 	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
 	statfs_cmd.oneline =
 		_("statistics on the filesystem of the currently open file");
+	statfs_cmd.help = statfs_help;
 
 	add_command(&stat_cmd);
 	add_command(&statx_cmd);
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 1103dc42..32bdd866 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1240,6 +1240,23 @@ Selected statistics from
 .BR statfs (2)
 and the XFS_IOC_FSGEOMETRY
 system call on the filesystem where the current file resides.
+.RS 1.0i
+.PD 0
+.TP
+.BI \-a
+Display statfs, geometry, and fs summary counter data.
+.TP
+.BI \-c
+Display fs summary counter data.
+.TP
+.BI \-g
+Display geometry data.
+.TP
+.BI \-s
+Display statfs data.
+.TP
+.RE
+.PD
 .TP
 .BI "inode  [ [ -n ] " number " ] [ -v ]"
 The inode command queries physical information about an inode. With