diff mbox

xfs_io: changes to statx interface [ver #2]

Message ID 29458.1490711932@warthog.procyon.org.uk (mailing list archive)
State Superseded, archived
Headers show

Commit Message

David Howells March 28, 2017, 2:38 p.m. UTC
Here are my current changes to Eric's statx interface patch.  I've made it
analoguous to the stat command and so it only does statx-of-fd.

I've made the "-m" flag able to take the words "basic" and "all" in
preference to a number and able to take an octal or hex number as an
alternative too.

I've dropped the -A and -L flags since it no longer passes a path over.

Finally, I've added a -c flag that causes an fstat() to be done as well and
the buffers compared as a consistency check.

David
---
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Howells March 28, 2017, 2:41 p.m. UTC | #1
David Howells <dhowells@redhat.com> wrote:

> Here are my current changes to Eric's statx interface patch.  I've made it
> analoguous to the stat command and so it only does statx-of-fd.
> 
> I've made the "-m" flag able to take the words "basic" and "all" in
> preference to a number and able to take an octal or hex number as an
> alternative too.
> 
> I've dropped the -A and -L flags since it no longer passes a path over.
> 
> Finally, I've added a -c flag that causes an fstat() to be done as well and
> the buffers compared as a consistency check.

Note that the intention is to put the testing of the syscall parameter
handling into LTP, along with testing of the symlink following and dirfd usage
since xfstests seems unsuitable for this.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen March 28, 2017, 4:42 p.m. UTC | #2
I am (literally) in the woods this week so can't really comment at length, but generally:

xfs_io is useful to facilitate testing at times, but usually doesn't have tests built into the tool itself.

And I think you are right that it is a poor fit for some of the testing you'd like to do.  There is a src/ dir in xfstests for specialized C tests which get called by the test harness; that might also be a reasonable option.

Thanks, 
Eric

> On Mar 28, 2017, at 9:41 AM, David Howells <dhowells@redhat.com> wrote:
> 
> Note that the intention is to put the testing of the syscall parameter
> handling into LTP, along with testing of the symlink following and dirfd usage
> since xfstests seems unsuitable for this.
> 
> David

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amir Goldstein March 28, 2017, 5:35 p.m. UTC | #3
On Tue, Mar 28, 2017 at 12:42 PM, Eric Sandeen <sandeen@sandeen.net> wrote:
> I am (literally) in the woods this week so can't really comment at length, but generally:
>
> xfs_io is useful to facilitate testing at times, but usually doesn't have tests built into the tool itself.
>

David,

Eric being the maintainer, so he gets to decide, but he is being
somewhat subtle.
There is no precedence AFAIK to tests within xfs_io.
Instead of comparing to stat with -c flag, you can make sure that
output of xfs_io -c 'statx -c'
is fully compatible to output of xfs_io -c 'stat -v' and do the test in scripts.

> And I think you are right that it is a poor fit for some of the testing you'd like to do.
> There is a src/ dir in xfstests for specialized C tests which get called by the test harness; that might also be a reasonable option.
>

That's actually the shortest path for you and there are quite a few
tests in xfstests that took
this path. It should be easy for you to copy & paste one of them.

That's not instead of adding xfs_io statx - just for tests that are
not natural to do with xfs_io.

> Thanks,
> Eric
>
>> On Mar 28, 2017, at 9:41 AM, David Howells <dhowells@redhat.com> wrote:
>>
>> Note that the intention is to put the testing of the syscall parameter
>> handling into LTP, along with testing of the symlink following and dirfd usage
>> since xfstests seems unsuitable for this.
>>
>> David
>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Howells March 28, 2017, 5:56 p.m. UTC | #4
Amir Goldstein <amir73il@gmail.com> wrote:

> That's actually the shortest path for you and there are quite a few
> tests in xfstests that took
> this path. It should be easy for you to copy & paste one of them.

Don't forget that syscall testing for this has to be added to LTP anyway...

David
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amir Goldstein March 28, 2017, 6:11 p.m. UTC | #5
On Tue, Mar 28, 2017 at 1:56 PM, David Howells <dhowells@redhat.com> wrote:
> Amir Goldstein <amir73il@gmail.com> wrote:
>
>> That's actually the shortest path for you and there are quite a few
>> tests in xfstests that took
>> this path. It should be easy for you to copy & paste one of them.
>
> Don't forget that syscall testing for this has to be added to LTP anyway...
>

Sure, LTP is good for that and many testers run LTP, but I have no idea how
wide is the file system diversity of their setups.
statx may have bugs in syscall/vfs and it may have bugs in specific
fs, so having
some C tests in LTP and some C tests in xfstest can't hurt.

It's not difficult to harness a C test to xfstest.
See recent example of C only (mostly) test in
 2161614 generic: concurrent non-overlapping direct I/O on the same extents
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Dilger March 28, 2017, 6:40 p.m. UTC | #6
On Mar 28, 2017, at 8:38 AM, David Howells <dhowells@redhat.com> wrote:
> 
> Here are my current changes to Eric's statx interface patch.  I've made it
> analoguous to the stat command and so it only does statx-of-fd.
> 
> I've made the "-m" flag able to take the words "basic" and "all" in
> preference to a number and able to take an octal or hex number as an
> alternative too.
> 
> I've dropped the -A and -L flags since it no longer passes a path over.
> 
> Finally, I've added a -c flag that causes an fstat() to be done as well and
> the buffers compared as a consistency check.

It probably makes sense to use "-C" to avoid confusion with xfs_io's "-c"
option, and also the "stat -c" option to dump only specific fields (which
may be useful to add at some point in the future as well).

Cheers, Andreas

> 
> David
> ---
> diff --git a/io/stat.c b/io/stat.c
> index a7aebcd..961b6d1 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -189,12 +189,10 @@ statx_help(void)
> " Display extended file status.\n"
> "\n"
> " Options:\n"
> -" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
> -" -A -- Suppress terminal automount traversal\n"
> +" -c -- Compare against fstat/fstatat on the same file/fd\n"
> +" -m mask -- Specify the field mask for the statx call (can also be 'basic' or 'all'; default STATX_ALL)\n"
> " -D -- Don't sync attributes with the server\n"
> " -F -- Force the attributes to be sync'd with the server\n"
> -" -L -- Follow symlinks (statx link target)\n"
> -" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"
> "\n"));
> }
> 
> @@ -227,6 +225,65 @@ dump_statx(struct statx *stx)
> }
> 
> /*
> + * Compare the contents of a statx struct with that of a stat struct and check
> + * that they're the same.
> + */
> +static int
> +cmp_statx(const struct statx *stx, const struct stat *st)
> +{
> +	const char *what = NULL;
> +
> +#define cmp(x) \
> +	do {					\
> +		what = #x;		\
> +		if (stx->stx_##x != st->st_##x)	\
> +			goto mismatch;		\
> +	} while (0)
> +
> +	cmp(blksize);
> +	cmp(nlink);
> +	cmp(uid);
> +	cmp(gid);
> +	cmp(mode);
> +	cmp(ino);
> +	cmp(size);
> +	cmp(blocks);
> +
> +#define devcmp(x) \
> +	do {						\
> +		what = #x".major";			\
> +		if (stx->stx_##x##_major != major(st->st_##x))	\
> +			goto mismatch;			\
> +		what = #x".minor";			\
> +		if (stx->stx_##x##_minor != minor(st->st_##x))	\
> +			goto mismatch;			\
> +	} while (0)
> +
> +	devcmp(dev);
> +	devcmp(rdev);
> +
> +#define timecmp(x) \
> +	do {						\
> +		what = #x".tv_sec";			\
> +		if (stx->stx_##x##time.tv_sec != st->st_##x##tim.tv_sec)	\
> +			goto mismatch;			\
> +		what = #x".tv_nsec";			\
> +		if (stx->stx_##x##time.tv_nsec != st->st_##x##tim.tv_nsec)	\
> +			goto mismatch;			\
> +	} while (0)
> +
> +	timecmp(a);
> +	timecmp(c);
> +	timecmp(m);
> +
> +	return 0;
> +
> +mismatch:
> +	fprintf(stderr, "Mismatch between stat and statx output (%s)\n", what);
> +	return -1;
> +}
> +
> +/*
>  * options:
>  * 	- input flags - query type
>  * 	- output style for flags (and all else?) (chars vs. hex?)
> @@ -239,16 +296,23 @@ statx_f(
> {
> 	int		c;
> 	struct statx	stx;
> -	int		atflag = AT_SYMLINK_NOFOLLOW;
> -	unsigned int	m_mask = 0;	/* mask requested with -m */
> -	int		Oflag = 0, mflag = 0;	/* -O or -m was used */
> +	struct stat	st;
> +	int		atflag = 0;
> 	unsigned int	mask = STATX_ALL;
> +	int		compare = 0;
> 
> -	while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
> +	while ((c = getopt(argc, argv, "cm:FD")) != EOF) {
> 		switch (c) {
> +		case 'c':
> +			compare = 1;
> +			break;
> 		case 'm':
> -			m_mask = atoi(optarg);
> -			mflag = 1;
> +			if (strcmp(optarg, "basic") == 0)
> +				mask = STATX_BASIC_STATS;
> +			else if (strcmp(optarg, "all") == 0)
> +				mask = STATX_ALL;
> +			else
> +				mask = strtoul(optarg, NULL, 0);
> 			break;
> 		case 'F':
> 			atflag &= ~AT_STATX_SYNC_TYPE;
> @@ -258,38 +322,28 @@ statx_f(
> 			atflag &= ~AT_STATX_SYNC_TYPE;
> 			atflag |= AT_STATX_DONT_SYNC;
> 			break;
> -		case 'L':
> -			atflag &= ~AT_SYMLINK_NOFOLLOW;
> -			break;
> -		case 'O':
> -			mask = STATX_BASIC_STATS;
> -			Oflag = 1;
> -			break;
> -		case 'A':
> -			atflag |= AT_NO_AUTOMOUNT;
> -			break;
> 		default:
> 			return command_usage(&statx_cmd);
> 		}
> 	}
> 
> -	if (Oflag && mflag) {
> -		printf("Cannot specify both -m mask and -O\n");
> -		return 0;
> -	}
> -
> -	/* -m overrides any other mask options */
> -	if (mflag)
> -		mask = m_mask;
> -
> 	memset(&stx, 0xbf, sizeof(stx));
> -	if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
> +
> +	if (statx(file->fd, NULL, atflag, mask, &stx) < 0) {
> 		perror("statx");
> 		return 0;
> 	}
> 
> -	dump_statx(&stx);
> +	if (compare) {
> +		if (fstat(file->fd, &st) < 0) {
> +			perror("fstat");
> +			return 0;
> +		}
> 
> +		cmp_statx(&stx, &st);
> +	}
> +
> +	dump_statx(&stx);
> 	return 0;
> }
> 
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 77ba760..d82f882 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -884,16 +884,23 @@ and the XFS_IOC_GETXATTR system call on the current file. If the
> option is specified, the atime (last access), mtime
> (last modify), and ctime (last change) timestamps are also displayed.
> .TP
> -.BR statx " [ " \-O " | " "\-m mask" " ][ \-" FDLA " ]"
> +.BR statx " [ " "\-m mask" " ][ \-" cFD " ]"
> Extended information from the statx syscall.
> .RS 1.0i
> .PD 0
> .TP 0.4i
> .B \-m mask
> -Specify the field mask for the statx call (default STATX_ALL)
> +Specify the field mask for the statx call as an decimal, hex or octal integer
> +or
> +.RI \" basic "\" or \"" all \"
> +to specify the basic stats that
> +.IR stat ()
> +returns or all the stats known by the header file.  All is the default.
> .TP
> -.B \-O
> -Add only basic stats (STATX_BASIC_STATS) to default mask
> +.B \-c
> +Do an
> +.IR fstat ()
> +call as well and compare the buffers.
> .TP
> .B \-F
> Force the attributes to be sync'd with the server
> @@ -901,12 +908,6 @@ Force the attributes to be sync'd with the server
> .B \-D
> Don't sync attributes with the server
> .TP
> -.B \-L
> -Follow symlinks (statx link target)
> -.TP
> -.B \-A
> -Suppress terminal automount traversal
> -.TP
> .RE
> .IP
> .TP


Cheers, Andreas
Amir Goldstein March 28, 2017, 7:07 p.m. UTC | #7
On Tue, Mar 28, 2017 at 2:40 PM, Andreas Dilger <adilger@dilger.ca> wrote:
> On Mar 28, 2017, at 8:38 AM, David Howells <dhowells@redhat.com> wrote:
>>
>> Here are my current changes to Eric's statx interface patch.  I've made it
>> analoguous to the stat command and so it only does statx-of-fd.
>>
>> I've made the "-m" flag able to take the words "basic" and "all" in
>> preference to a number and able to take an octal or hex number as an
>> alternative too.
>>
>> I've dropped the -A and -L flags since it no longer passes a path over.
>>
>> Finally, I've added a -c flag that causes an fstat() to be done as well and
>> the buffers compared as a consistency check.
>
> It probably makes sense to use "-C" to avoid confusion with xfs_io's "-c"
> option, and also the "stat -c" option to dump only specific fields (which
> may be useful to add at some point in the future as well).
>

I'm afraid its not better for avoiding confusion.
-C already taken as well for new "oneshot commands"...
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen March 28, 2017, 8:36 p.m. UTC | #8
On 3/28/17 12:35 PM, Amir Goldstein wrote:
> On Tue, Mar 28, 2017 at 12:42 PM, Eric Sandeen <sandeen@sandeen.net> wrote:
>> I am (literally) in the woods this week so can't really comment at length, but generally:
>>
>> xfs_io is useful to facilitate testing at times, but usually doesn't have tests built into the tool itself.
>>
> 
> David,
> 
> Eric being the maintainer, so he gets to decide, but he is being
> somewhat subtle.

Sorry about that.  More like: terse, with only a phone to communicate,
and a wife who thinks we're on vacation.  ;)

> There is no precedence AFAIK to tests within xfs_io.
> Instead of comparing to stat with -c flag, you can make sure that
> output of xfs_io -c 'statx -c'
> is fully compatible to output of xfs_io -c 'stat -v' and do the test in scripts.

*nod*

>> And I think you are right that it is a poor fit for some of the testing you'd like to do.
>> There is a src/ dir in xfstests for specialized C tests which get called by the test harness; that might also be a reasonable option.
>>
> 
> That's actually the shortest path for you and there are quite a few
> tests in xfstests that took
> this path. It should be easy for you to copy & paste one of them.
> 
> That's not instead of adding xfs_io statx - just for tests that are
> not natural to do with xfs_io.

Agreed.

I think there's value in having a statx command in xfs_io, but it won't
be able to facilitate testing /everything/ that needs to be tested.

Thanks,
-Eric

>> Thanks,
>> Eric
>>
>>> On Mar 28, 2017, at 9:41 AM, David Howells <dhowells@redhat.com> wrote:
>>>
>>> Note that the intention is to put the testing of the syscall parameter
>>> handling into LTP, along with testing of the symlink following and dirfd usage
>>> since xfstests seems unsuitable for this.
>>>
>>> David
>>
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen March 28, 2017, 10:12 p.m. UTC | #9
On 3/28/17 2:07 PM, Amir Goldstein wrote:
> On Tue, Mar 28, 2017 at 2:40 PM, Andreas Dilger <adilger@dilger.ca> wrote:
>> On Mar 28, 2017, at 8:38 AM, David Howells <dhowells@redhat.com> wrote:
>>>
>>> Here are my current changes to Eric's statx interface patch.  I've made it
>>> analoguous to the stat command and so it only does statx-of-fd.
>>>
>>> I've made the "-m" flag able to take the words "basic" and "all" in
>>> preference to a number and able to take an octal or hex number as an
>>> alternative too.
>>>
>>> I've dropped the -A and -L flags since it no longer passes a path over.
>>>
>>> Finally, I've added a -c flag that causes an fstat() to be done as well and
>>> the buffers compared as a consistency check.
>>
>> It probably makes sense to use "-C" to avoid confusion with xfs_io's "-c"
>> option, and also the "stat -c" option to dump only specific fields (which
>> may be useful to add at some point in the future as well).
>>
> 
> I'm afraid its not better for avoiding confusion.
> -C already taken as well for new "oneshot commands"...

There's no conflict here AFAICT; the "-C" for oneshot is for
xfs_io invocation; individual commands are free to re-use such
options for their own purposes.

i.e.:

# xfs_io -C "command -C" file

is fine; the two different -C args have different contexts.  Adding a
-C to the statx command poses no problem.

And with that public service announcement let me go read this thread and
catch up... ;)

-Eric

> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Chinner March 28, 2017, 11:18 p.m. UTC | #10
On Tue, Mar 28, 2017 at 03:38:52PM +0100, David Howells wrote:
> Here are my current changes to Eric's statx interface patch.  I've made it
> analoguous to the stat command and so it only does statx-of-fd.
> 
> I've made the "-m" flag able to take the words "basic" and "all" in
> preference to a number and able to take an octal or hex number as an
> alternative too.
> 
> I've dropped the -A and -L flags since it no longer passes a path over.
> 
> Finally, I've added a -c flag that causes an fstat() to be done as well and
> the buffers compared as a consistency check.
> 
> David
> ---
> diff --git a/io/stat.c b/io/stat.c
> index a7aebcd..961b6d1 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -189,12 +189,10 @@ statx_help(void)
>  " Display extended file status.\n"
>  "\n"
>  " Options:\n"
> -" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
> -" -A -- Suppress terminal automount traversal\n"
> +" -c -- Compare against fstat/fstatat on the same file/fd\n"

Running "compare" tests like this is not a function that xfs_io
should perform. Have it run and output the statx information,
and use xfstests to compare that against a golden output and/or
the output of the xfs_io stat command.

i.e. xfs_io is a utility for executing commands, whilst xfstests is
used to test that the executed functionality has the correct output.

Cheers,

Dave.
David Howells March 29, 2017, 3:24 p.m. UTC | #11
Dave Chinner <david@fromorbit.com> wrote:

> Running "compare" tests like this is not a function that xfs_io
> should perform. Have it run and output the statx information,
> and use xfstests to compare that against a golden output and/or
> the output of the xfs_io stat command.

A golden output that covers timestamps isn't possible.

Also comparing the output of Eric's statx command with the output of the stat
command as it stands isn't practical.  I guess I can add another parameter to
the stat command to produce something that looks the same.

I'm tempted to write C-based python3 module that gives raw access to the
system calls (something I want for the unionmount testsuite) and then script
this in python.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/io/stat.c b/io/stat.c
index a7aebcd..961b6d1 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -189,12 +189,10 @@  statx_help(void)
 " Display extended file status.\n"
 "\n"
 " Options:\n"
-" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
-" -A -- Suppress terminal automount traversal\n"
+" -c -- Compare against fstat/fstatat on the same file/fd\n"
+" -m mask -- Specify the field mask for the statx call (can also be 'basic' or 'all'; default STATX_ALL)\n"
 " -D -- Don't sync attributes with the server\n"
 " -F -- Force the attributes to be sync'd with the server\n"
-" -L -- Follow symlinks (statx link target)\n"
-" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"
 "\n"));
 }
 
@@ -227,6 +225,65 @@  dump_statx(struct statx *stx)
 }
 
 /*
+ * Compare the contents of a statx struct with that of a stat struct and check
+ * that they're the same.
+ */
+static int
+cmp_statx(const struct statx *stx, const struct stat *st)
+{
+	const char *what = NULL;
+
+#define cmp(x) \
+	do {					\
+		what = #x;		\
+		if (stx->stx_##x != st->st_##x)	\
+			goto mismatch;		\
+	} while (0)
+
+	cmp(blksize);
+	cmp(nlink);
+	cmp(uid);
+	cmp(gid);
+	cmp(mode);
+	cmp(ino);
+	cmp(size);
+	cmp(blocks);
+
+#define devcmp(x) \
+	do {						\
+		what = #x".major";			\
+		if (stx->stx_##x##_major != major(st->st_##x))	\
+			goto mismatch;			\
+		what = #x".minor";			\
+		if (stx->stx_##x##_minor != minor(st->st_##x))	\
+			goto mismatch;			\
+	} while (0)
+
+	devcmp(dev);
+	devcmp(rdev);
+
+#define timecmp(x) \
+	do {						\
+		what = #x".tv_sec";			\
+		if (stx->stx_##x##time.tv_sec != st->st_##x##tim.tv_sec)	\
+			goto mismatch;			\
+		what = #x".tv_nsec";			\
+		if (stx->stx_##x##time.tv_nsec != st->st_##x##tim.tv_nsec)	\
+			goto mismatch;			\
+	} while (0)
+
+	timecmp(a);
+	timecmp(c);
+	timecmp(m);
+
+	return 0;
+
+mismatch:
+	fprintf(stderr, "Mismatch between stat and statx output (%s)\n", what);
+	return -1;
+}
+
+/*
  * options:
  * 	- input flags - query type
  * 	- output style for flags (and all else?) (chars vs. hex?)
@@ -239,16 +296,23 @@  statx_f(
 {
 	int		c;
 	struct statx	stx;
-	int		atflag = AT_SYMLINK_NOFOLLOW;
-	unsigned int	m_mask = 0;	/* mask requested with -m */
-	int		Oflag = 0, mflag = 0;	/* -O or -m was used */
+	struct stat	st;
+	int		atflag = 0;
 	unsigned int	mask = STATX_ALL;
+	int		compare = 0;
 
-	while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
+	while ((c = getopt(argc, argv, "cm:FD")) != EOF) {
 		switch (c) {
+		case 'c':
+			compare = 1;
+			break;
 		case 'm':
-			m_mask = atoi(optarg);
-			mflag = 1;
+			if (strcmp(optarg, "basic") == 0)
+				mask = STATX_BASIC_STATS;
+			else if (strcmp(optarg, "all") == 0)
+				mask = STATX_ALL;
+			else
+				mask = strtoul(optarg, NULL, 0);
 			break;
 		case 'F':
 			atflag &= ~AT_STATX_SYNC_TYPE;
@@ -258,38 +322,28 @@  statx_f(
 			atflag &= ~AT_STATX_SYNC_TYPE;
 			atflag |= AT_STATX_DONT_SYNC;
 			break;
-		case 'L':
-			atflag &= ~AT_SYMLINK_NOFOLLOW;
-			break;
-		case 'O':
-			mask = STATX_BASIC_STATS;
-			Oflag = 1;
-			break;
-		case 'A':
-			atflag |= AT_NO_AUTOMOUNT;
-			break;
 		default:
 			return command_usage(&statx_cmd);
 		}
 	}
 
-	if (Oflag && mflag) {
-		printf("Cannot specify both -m mask and -O\n");
-		return 0;
-	}
-
-	/* -m overrides any other mask options */
-	if (mflag)
-		mask = m_mask;
-
 	memset(&stx, 0xbf, sizeof(stx));
-	if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
+
+	if (statx(file->fd, NULL, atflag, mask, &stx) < 0) {
 		perror("statx");
 		return 0;
 	}
 
-	dump_statx(&stx);
+	if (compare) {
+		if (fstat(file->fd, &st) < 0) {
+			perror("fstat");
+			return 0;
+		}
 
+		cmp_statx(&stx, &st);
+	}
+	
+	dump_statx(&stx);
 	return 0;
 }
 
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 77ba760..d82f882 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -884,16 +884,23 @@  and the XFS_IOC_GETXATTR system call on the current file. If the
 option is specified, the atime (last access), mtime
 (last modify), and ctime (last change) timestamps are also displayed.
 .TP
-.BR statx " [ " \-O " | " "\-m mask" " ][ \-" FDLA " ]"
+.BR statx " [ " "\-m mask" " ][ \-" cFD " ]"
 Extended information from the statx syscall.
 .RS 1.0i
 .PD 0
 .TP 0.4i
 .B \-m mask
-Specify the field mask for the statx call (default STATX_ALL)
+Specify the field mask for the statx call as an decimal, hex or octal integer
+or
+.RI \" basic "\" or \"" all \"
+to specify the basic stats that
+.IR stat ()
+returns or all the stats known by the header file.  All is the default.
 .TP
-.B \-O
-Add only basic stats (STATX_BASIC_STATS) to default mask
+.B \-c
+Do an
+.IR fstat ()
+call as well and compare the buffers.
 .TP
 .B \-F
 Force the attributes to be sync'd with the server
@@ -901,12 +908,6 @@  Force the attributes to be sync'd with the server
 .B \-D
 Don't sync attributes with the server
 .TP
-.B \-L
-Follow symlinks (statx link target)
-.TP
-.B \-A
-Suppress terminal automount traversal
-.TP
 .RE
 .IP
 .TP