linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xfs: report crtime and attribute flags to statx
@ 2017-03-07  0:06 Darrick J. Wong
  2017-03-07  5:01 ` Christoph Hellwig
  2017-03-07 17:22 ` statx manpage David Howells
  0 siblings, 2 replies; 38+ messages in thread
From: Darrick J. Wong @ 2017-03-07  0:06 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: dhowells, linux-fsdevel, xfs

statx has the ability to report inode creation times and inode flags, so
hook up di_crtime and di_flags to that functionality.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_iops.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 229cc6a..ebfc133 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -516,6 +516,20 @@ xfs_vn_getattr(
 	stat->blocks =
 		XFS_FSB_TO_BB(mp, ip->i_d.di_nblocks + ip->i_delayed_blks);
 
+	if (ip->i_d.di_version == 3) {
+		if (request_mask & STATX_BTIME) {
+			stat->result_mask |= STATX_BTIME;
+			stat->btime.tv_sec = ip->i_d.di_crtime.t_sec;
+			stat->btime.tv_nsec = ip->i_d.di_crtime.t_nsec;
+		}
+	}
+
+	if (ip->i_d.di_flags & XFS_DIFLAG_IMMUTABLE)
+		stat->attributes |= STATX_ATTR_IMMUTABLE;
+	if (ip->i_d.di_flags & XFS_DIFLAG_APPEND)
+		stat->attributes |= STATX_ATTR_APPEND;
+	if (ip->i_d.di_flags & XFS_DIFLAG_NODUMP)
+		stat->attributes |= STATX_ATTR_NODUMP;
 
 	switch (inode->i_mode & S_IFMT) {
 	case S_IFBLK:

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH] xfs: report crtime and attribute flags to statx
  2017-03-07  0:06 [PATCH] xfs: report crtime and attribute flags to statx Darrick J. Wong
@ 2017-03-07  5:01 ` Christoph Hellwig
  2017-03-07 17:23   ` Darrick J. Wong
  2017-03-07 17:22 ` statx manpage David Howells
  1 sibling, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2017-03-07  5:01 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: dhowells, linux-fsdevel, xfs

On Mon, Mar 06, 2017 at 04:06:09PM -0800, Darrick J. Wong wrote:
> statx has the ability to report inode creation times and inode flags, so
> hook up di_crtime and di_flags to that functionality.

This would be great to have in 4.11 together with the initial statx
implementation.  But until I see documentation and testcases for statx
I don't really feel comfortable reviewing anything related to it.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* statx manpage
  2017-03-07  0:06 [PATCH] xfs: report crtime and attribute flags to statx Darrick J. Wong
  2017-03-07  5:01 ` Christoph Hellwig
@ 2017-03-07 17:22 ` David Howells
  2017-03-07 18:02   ` Darrick J. Wong
                     ` (11 more replies)
  1 sibling, 12 replies; 38+ messages in thread
From: David Howells @ 2017-03-07 17:22 UTC (permalink / raw)
  To: Christoph Hellwig, mtk.manpages
  Cc: dhowells, Darrick J. Wong, linux-fsdevel, xfs

Christoph Hellwig <hch@infradead.org> wrote:

> This would be great to have in 4.11 together with the initial statx
> implementation.  But until I see documentation and testcases for statx
> I don't really feel comfortable reviewing anything related to it.

Well, since you asked for documentation, here's a manual page for you to
review:-)

Note that as it isn't in glibc yet, I've left out all the
set-this-and-that-#define-to-make-it-appear stuff except where it is pertinent
to particular constants.

I don't suppose you know where the documentation on writing xfstests tests is?
xfstests-dev/doc/ only contains an old and out of date changelog.

David
---
'\" t
.\" Copyright (c) 1992 Drew Eckhardt (drew@cs.colorado.edu), March 28, 1992
.\" Parts Copyright (c) 1995 Nicolai Langfeldt (janl@ifi.uio.no), 1/1/95
.\" and Copyright (c) 2006, 2007, 2014 Michael Kerrisk <mtk.manpages@gmail.com>
.\" and Copyright (c) 2017 David Howells <dhowells@redhat.com>
.\"
.\" %%%LICENSE_START(VERBATIM)
.\" Permission is granted to make and distribute verbatim copies of this
.\" manual provided the copyright notice and this permission notice are
.\" preserved on all copies.
.\"
.\" Permission is granted to copy and distribute modified versions of this
.\" manual under the conditions for verbatim copying, provided that the
.\" entire resulting derived work is distributed under the terms of a
.\" permission notice identical to this one.
.\"
.\" Since the Linux kernel and libraries are constantly changing, this
.\" manual page may be incorrect or out-of-date.  The author(s) assume no
.\" responsibility for errors or omissions, or for damages resulting from
.\" the use of the information contained herein.  The author(s) may not
.\" have taken the same level of care in the production of this manual,
.\" which is licensed free of charge, as they might when working
.\" professionally.
.\"
.\" Formatted or processed versions of this manual, if unaccompanied by
.\" the source, must acknowledge the copyright and authors of this work.
.\" %%%LICENSE_END
.\"
.TH STATX 2 2017-03-07 "Linux" "Linux Programmer's Manual"
.SH NAME
statx \- Get file status (extended)
.SH SYNOPSIS
.nf
.B #include <sys/types.h>
.br
.B #include <sys/stat.h>
.br
.B #include <unistd.h>
.br
.BR "#include <fcntl.h>           " "/* Definition of AT_* constants */"
.sp
.BI "int statx(int " dirfd ", const char *" pathname ", int " flags ","
.BI "          unsigned int " mask ", struct statx *" buf );
.fi
.sp
.in -4n
Feature Test Macro Requirements for glibc (see
.BR feature_test_macros (7)):
.in
.ad l
.PD 0
.sp
.RS 4
<unknown as yet>
.RE
.PD
.ad
.SH DESCRIPTION
.PP
This function returns information about a file, storing it in the buffer
pointed to by
.IR buf .
The buffer is filled in according to the following type:
.PP
.in +4n
.nf
struct statx {
    __u32 stx_mask;          -- Mask of bits indicating filled fields
    __u32 stx_blksize;       -- Block size for filesystem I/O
    __u64 stx_attributes;    -- Extra file attribute indicators
    __u32 stx_nlink;         -- Number of hard links
    __u32 stx_uid;           -- User ID of owner
    __u32 stx_gid;           -- Group ID of owner
    __u16 stx_mode;          -- File type and mode
    __u64 stx_ino;           -- Inode number
    __u64 stx_size;          -- Total size in bytes
    __u64 stx_blocks;        -- Number of 512B blocks allocated
    struct statx_timestamp stx_atime;  -- Time of last access
    struct statx_timestamp stx_btime;  -- Time of creation
    struct statx_timestamp stx_ctime;  -- Time of last status change
    struct statx_timestamp stx_mtime;  -- Time of last modification
    __u32 stx_rdev_major;    } Device number if device file
    __u32 stx_rdev_minor;    }
    __u32 stx_dev_major;      } Device number of containing file
    __u32 stx_dev_minor;      }
};
.fi
.in
.PP
Where the timestamps are defined as:
.PP
.in +4n
.nf
struct statx_timestamp {
    __s64 tv_sec;    -- Number of seconds before or since 1970
    __s32 tv_nsec;   -- Number of nanoseconds before or since tv_sec
};
.fi
.in
.PP
(Note that reserved space and padding is ommitted)
.SS
Invoking \fBstatx\fR():
.PP
To access a file's status, no permissions are required on the file itself, but
in the case of
.BR statx ()
with a path, execute (search) permission is required on all of the directories
in
.I pathname
that lead to the file.
.PP
.BR statx ()
uses
.IR pathname ", " dirfd " and " flags
to locate the target file in one of a variety of ways:
.TP
[*] By absolute path.
.I pathname
points to an absolute path and
.I dirfd
is ignored.  The file is looked up by name, starting from the root of the
filesystem as seen by the calling process.
.TP
[*] By cwd-relative path.
.I pathname
points to a relative path and
.IR dirfd " is " AT_FDCWD .
The file is looked up by name, starting from the current working directory.
.TP
[*] By dir-relative path.
.I pathname
points to relative path and
.I dirfd
indicates a file descriptor pointing to a directory.  The file is looked up by
name, starting from the directory specified by
.IR dirfd .
.TP
[*] By file descriptor.
.IR pathname " is " NULL " and " dirfd
indicates a file descriptor.  The file attached to the file descriptor is
queried directly.  The file descriptor may point to any type of file, not just
a directory.
.PP
.I flags
can be used to influence a path-based lookup.  A value for
.I flags
is constructed by OR'ing together zero or more of the following constants:
.TP
.BR AT_EMPTY_PATH " (since Linux 2.6.39)"
.\" commit 65cfc6722361570bfe255698d9cd4dccaf47570d
If
.I pathname
is an empty string, operate on the file referred to by
.IR dirfd
(which may have been obtained using the
.BR open (2)
.B O_PATH
flag).
If
.I dirfd
is
.BR AT_FDCWD ,
the call operates on the current working directory.
In this case,
.I dirfd
can refer to any type of file, not just a directory.
This flag is Linux-specific; define
.B _GNU_SOURCE
.\" Before glibc 2.16, defining _ATFILE_SOURCE sufficed
to obtain its definition.
.TP
.BR AT_NO_AUTOMOUNT " (since Linux 2.6.38)"
Don't automount the terminal ("basename") component of
.I pathname
if it is a directory that is an automount point.
This allows the caller to gather attributes of an automount point
(rather than the location it would mount).
This flag can be used in tools that scan directories
to prevent mass-automounting of a directory of automount points.
The
.B AT_NO_AUTOMOUNT
flag has no effect if the mount point has already been mounted over.
This flag is Linux-specific; define
.B _GNU_SOURCE
.\" Before glibc 2.16, defining _ATFILE_SOURCE sufficed
to obtain its definition.
.TP
.B AT_SYMLINK_NOFOLLOW
If
.I pathname
is a symbolic link, do not dereference it:
instead return information about the link itself, like
.BR lstat ().
.PP
.I flags
can also be used to control what sort of synchronisation the kernel will do
when querying a file on a remote filesystem.  This is done by OR'ing in one of
the following values:
.TP
AT_STATX_SYNC_AS_STAT
Do whatever
.BR stat ()
does.  This is the default and is very much filesystem specific.
.TP
AT_STATX_FORCE_SYNC
Force the attributes to be synchronised with the server.  This may require that
a network filesystem perform a data writeback to get the timestamps correct.
.TP
AT_STATX_DONT_SYNC
Don't synchronise anything, but rather just take whatever the system has cached
if possible.  This may mean that the information returned is approximate, but,
on a network filesystem, it may not involve a round trip to the server - even
if no lease is held.
.PP
The
.I mask
argument to
.BR statx ()
is used to tell the kernel which fields the caller is interested in
.I mask
is an OR'ed combination of the following constants:
.PP
.in +4n
.TS
lB l.
STATX_TYPE	Want stx_mode & S_IFMT
STATX_MODE	Want stx_mode & ~S_IFMT
STATX_NLINK	Want stx_nlink
STATX_UID	Want stx_uid
STATX_GID	Want stx_gid
STATX_ATIME	Want stx_atime{,_ns}
STATX_MTIME	Want stx_mtime{,_ns}
STATX_CTIME	Want stx_ctime{,_ns}
STATX_INO	Want stx_ino
STATX_SIZE	Want stx_size
STATX_BLOCKS	Want stx_blocks
STATX_BASIC_STATS	[The stuff in the normal stat struct]
STATX_BTIME	Want stx_btime{,_ns}
STATX_ALL	[All currently available stuff]
.TE
.in
.PP
.B "Do not"
simply set
.I mask
to UINT_MAX as one or more bits may, in future, be used to specify an extension
to the buffer.
.SS
The returned information
.PP
The status information for the target file is returned in the
.I statx
structure pointed to by
.IR buf .
Included in this is
.I stx_mask
which indicates what other information has been returned.
.I stx_mask
has the same format as the mask argument and bits are set in it to indicate
which fields have been filled in.
.PP
It should be noted that the kernel may return fields that weren't requested and
may fail to return fields that were requested, depending on what the backing
filesystem supports.  In either case,
.I stx_mask
will not be equal
.IR mask .
.PP
If a filesystem does not support a field or if it has an unrepresentable value
(for instance, a file with an exotic type), then the mask bit corresponding to
that field will be cleared in
.I stx_mask
even if the user asked for it and a dummy value will be filled in for
compatibility purposes if one is available (e.g. a dummy uid and gid may be
specified to mount under some circumstances).
.PP
A filesystem may also fill in fields that the caller didn't ask for if it has
values for them available at no extra cost.  If this happens, the corresponding
bits will be set in
.IR stx_mask .
.PP

.\" Background: inode attributes are modified with i_mutex held, but
.\" read by stat() without taking the mutex.
.I Note:
For performance and simplicity reasons, different fields in the
.I statx
structure may contain state information from different moments
during the execution of the system call.
For example, if
.IR stx_mode
or
.IR stx_uid
is changed by another process by calling
.BR chmod (2)
or
.BR chown (2),
.BR stat ()
might return the old
.I stx_mode
together with the new
.IR stx_uid ,
or the old
.I stx_uid
together with the new
.IR stx_mode .
.PP
Apart from stx_mask (which is described above), the fields in the
.I statx
structure are:
.TP
.I stx_mode
The file type and mode.  This is described in more detail below.
.TP
.I stx_size
The size of the file (if it is a regular file or a symbolic link) in bytes.
The size of a symbolic link is the length of the pathname it contains, without
a terminating null byte.
.TP
.I stx_blocks
The number of blocks allocated to the file on the medium, in 512-byte units.
(This may be smaller than
.IR stx_size /512
when the file has holes.)
.TP
.I stx_blksize
The "preferred" blocksize for efficient filesystem I/O.  (Writing to a file in
smaller chunks may cause an inefficient read-modify-rewrite.)
.TP
.I stx_nlink
The number of hard links on a file.
.TP
.I stx_uid
The user ID of the file's owner.
.TP
.I stx_gid
The ID of the group that may access the file.
.TP
.IR stx_dev_major " and "  stx_dev_minor
The device on which this file (inode) resides.
.TP
.IR stx_rdev_major " and "  stx_rdev_minor
The device that this file (inode) represents if the file is of block or
character device type.
.TP
.I stx_attributes
Further status information about the file.  This consists of zero or more of
the following constants OR'ed together:
.in +4n
.TS
lB l.
STATX_ATTR_COMPRESSED	File is compressed by the fs
STATX_ATTR_IMMUTABLE	File is marked immutable
STATX_ATTR_APPEND	File is append-only
STATX_ATTR_NODUMP	File is not to be dumped
STATX_ATTR_ENCRYPTED	File requires key to decrypt in fs
.TE
.in
.TP
.I stx_atime
The file's last access timestamp.
This field is changed by file accesses, for example, by
.BR execve (2),
.BR mknod (2),
.BR pipe (2),
.BR utime (2),
and
.BR read (2)
(of more than zero bytes).
Other routines, such as
.BR mmap (2),
may or may not update it.
.TP
.I stx_btime
The file's creation timestamp.  This is set on file creation and not changed
subsequently.
.TP
.I stx_ctime
The file's last status change timestamp.  This field is changed by writing or
by setting inode information (i.e., owner, group, link count, mode, etc.).
.TP
.I stx_mtime
The file's last modification timestamp.  This is changed by file modifications,
for example, by
.BR mknod (2),
.BR truncate (2),
.BR utime (2),
and
.BR write (2)
(of more than zero bytes).  Moreover, the modification time of a directory is
changed by the creation or deletion of files in that directory.  This field is
.I not
changed for changes in owner, group, hard link count, or mode.



.PP
Not all of the Linux filesystems implement all of the timestamp fields.  Some
filesystems allow mounting in such a way that file and/or directory accesses do
not cause an update of the
.I stx_atime
field.
(See
.IR noatime ,
.IR nodiratime ,
and
.I relatime
in
.BR mount (8),
and related information in
.BR mount (2).)
In addition,
.I stx_atime
is not updated if a file is opened with the
.BR O_NOATIME ;
see
.BR open (2).

.SS File type and mode
.PP
The
.I stx_mode
field contains the combined file type and mode.  POSIX refers to the bits in
this field corresponding to the mask
.B S_IFMT
(see below) as the
.IR "file type" ,
the 12 bits corresponding to the mask 07777 as the
.IR "file mode bits"
and the least significant 9 bits (0777) as the
.IR "file permission bits" .
.IP
The following mask values are defined for the file type of the
.I stx_mode
field:
.in +4n
.TS
lB l l.
S_IFMT	0170000	bit mask for the file type bit field

S_IFSOCK	0140000	socket
S_IFLNK	0120000	symbolic link
S_IFREG	0100000	regular file
S_IFBLK	0060000	block device
S_IFDIR	0040000	directory
S_IFCHR	0020000	character device
S_IFIFO	0010000	FIFO
.TE
.in
.IP
Note that
.I stx_mode
has two mask flags covering it: one for the type and one for the mode bits.
.PP
To test for a regular file (for example), one could write:
.nf
.in +4n
statx(AT_FDCWD, pathname, 0, STATX_BASIC_STATS, &sb);
if ((sb.stx_mode & S_IFMT) == S_IFREG) {
    /* Handle regular file */
}
.in
.fi
.PP
Because tests of the above form are common, additional macros are defined by
POSIX to allow the test of the file type in
.I stx_mode
to be written more concisely:
.RS 4
.TS
lB l.
\fBS_ISREG\fR(m)	Is it a regular file?
\fBS_ISDIR\fR(m)	Is it a directory?
\fBS_ISCHR\fR(m)	Is it a character device?
\fBS_ISBLK\fR(m)	Is it a block device?
\fBS_ISFIFO\fR(m)	Is it a FIFO (named pipe)?
\fBS_ISLNK\fR(m)	Is it a symbolic link?  (Not in POSIX.1-1996.)
\fBS_ISSOCK\fR(m)	Is it a socket?  (Not in POSIX.1-1996.)
.TE
.RE
.PP
The preceding code snippet could thus be rewritten as:

.nf
.in +4n
statx(AT_FDCWD, pathname, 0, STATX_BASIC_STATS, &sb);
if (S_ISREG(sb.stx_mode)) {
    /* Handle regular file */
}
.in
.fi
.PP
The definitions of most of the above file type test macros
are provided if any of the following feature test macros is defined:
.BR _BSD_SOURCE
(in glibc 2.19 and earlier),
.BR _SVID_SOURCE
(in glibc 2.19 and earlier),
or
.BR _DEFAULT_SOURCE
(in glibc 2.20 and later).
In addition, definitions of all of the above macros except
.BR S_IFSOCK
and
.BR S_ISSOCK ()
are provided if
.BR _XOPEN_SOURCE
is defined.
The definition of
.BR S_IFSOCK
can also be exposed by defining
.BR _XOPEN_SOURCE
with a value of 500 or greater.

The definition of
.BR S_ISSOCK ()
is exposed if any of the following feature test macros is defined:
.BR _BSD_SOURCE
(in glibc 2.19 and earlier),
.BR _DEFAULT_SOURCE
(in glibc 2.20 and later),
.BR _XOPEN_SOURCE
with a value of 500 or greater, or
.BR _POSIX_C_SOURCE
with a value of 200112L or greater.
.PP
The following mask values are defined for
the file mode component of the
.I stx_mode
field:
.in +4n
.TS
lB l l.
S_ISUID	  04000	set-user-ID bit
S_ISGID	  02000	set-group-ID bit (see below)
S_ISVTX	  01000	sticky bit (see below)

S_IRWXU	  00700	owner has read, write, and execute permission
S_IRUSR	  00400	owner has read permission
S_IWUSR	  00200	owner has write permission
S_IXUSR	  00100	owner has execute permission

S_IRWXG	  00070	group has read, write, and execute permission
S_IRGRP	  00040	group has read permission
S_IWGRP	  00020	group has write permission
S_IXGRP	  00010	group has execute permission

S_IRWXO	  00007	T{
others (not in group) have read, write, and execute permission
T}
S_IROTH	  00004	others have read permission
S_IWOTH	  00002	others have write permission
S_IXOTH	  00001	others have execute permission
.TE
.in
.P
The set-group-ID bit
.RB ( S_ISGID )
has several special uses.
For a directory, it indicates that BSD semantics is to be used
for that directory: files created there inherit their group ID from
the directory, not from the effective group ID of the creating process,
and directories created there will also get the
.B S_ISGID
bit set.
For a file that does not have the group execution bit
.RB ( S_IXGRP )
set,
the set-group-ID bit indicates mandatory file/record locking.
.P
The sticky bit
.RB ( S_ISVTX )
on a directory means that a file
in that directory can be renamed or deleted only by the owner
of the file, by the owner of the directory, and by a privileged
process.


.SH RETURN VALUE
On success, zero is returned.
On error, \-1 is returned, and
.I errno
is set appropriately.
.SH ERRORS
.TP
.B EINVAL
Invalid flag specified in
.IR flags .
.TP
.B EACCES
Search permission is denied for one of the directories
in the path prefix of
.IR pathname .
(See also
.BR path_resolution (7).)
.TP
.B EBADF
.I dirfd
is not a valid open file descriptor.
.TP
.B EFAULT
Bad address.
.TP
.B ELOOP
Too many symbolic links encountered while traversing the path.
.TP
.B ENAMETOOLONG
.I pathname
is too long.
.TP
.B ENOENT
A component of
.I pathname
does not exist, or
.I pathname
is an empty string.
.TP
.B ENOMEM
Out of memory (i.e., kernel memory).
.TP
.B ENOTDIR
A component of the path prefix of
.I pathname
is not a directory or
.I pathname
is relative and
.I dirfd
is a file descriptor referring to a file other than a directory.
.SH VERSIONS
.BR statx ()
was added to Linux in kernel 4.11;
library support is not yet added to glibc.
.SH SEE ALSO
.BR ls (1),
.BR stat (1),
.BR access (2),
.BR chmod (2),
.BR chown (2),
.BR readlink (2),
.BR utime (2),
.BR capabilities (7),
.BR symlink (7)

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] xfs: report crtime and attribute flags to statx
  2017-03-07  5:01 ` Christoph Hellwig
@ 2017-03-07 17:23   ` Darrick J. Wong
  0 siblings, 0 replies; 38+ messages in thread
From: Darrick J. Wong @ 2017-03-07 17:23 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: dhowells, linux-fsdevel, xfs

On Mon, Mar 06, 2017 at 09:01:40PM -0800, Christoph Hellwig wrote:
> On Mon, Mar 06, 2017 at 04:06:09PM -0800, Darrick J. Wong wrote:
> > statx has the ability to report inode creation times and inode flags, so
> > hook up di_crtime and di_flags to that functionality.
> 
> This would be great to have in 4.11 together with the initial statx
> implementation.  But until I see documentation and testcases for statx
> I don't really feel comfortable reviewing anything related to it.

Same feeling here -- aside from cargo-culting from the proposed ext4
patch I don't have much to go on other than floating a test balloon to
see if Mr. Howells responds.

(FWIW the samples/test-statx.c program reported the flags & btime.)

--D

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
@ 2017-03-07 18:02   ` Darrick J. Wong
  2017-03-07 18:39   ` David Howells
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 38+ messages in thread
From: Darrick J. Wong @ 2017-03-07 18:02 UTC (permalink / raw)
  To: David Howells; +Cc: Christoph Hellwig, mtk.manpages, linux-fsdevel, xfs

On Tue, Mar 07, 2017 at 05:22:55PM +0000, David Howells wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > This would be great to have in 4.11 together with the initial statx
> > implementation.  But until I see documentation and testcases for statx
> > I don't really feel comfortable reviewing anything related to it.
> 
> Well, since you asked for documentation, here's a manual page for you to
> review:-)

HA, ok.  This came in while I was scribbling a reply to Christoph's email.
I'll have a look.

> Note that as it isn't in glibc yet, I've left out all the
> set-this-and-that-#define-to-make-it-appear stuff except where it is pertinent
> to particular constants.
> 
> I don't suppose you know where the documentation on writing xfstests tests is?
> xfstests-dev/doc/ only contains an old and out of date changelog.

/me usually just copies one of the newer tests and changes it. <cough>

> David
> ---
> '\" t
> .\" Copyright (c) 1992 Drew Eckhardt (drew@cs.colorado.edu), March 28, 1992
> .\" Parts Copyright (c) 1995 Nicolai Langfeldt (janl@ifi.uio.no), 1/1/95
> .\" and Copyright (c) 2006, 2007, 2014 Michael Kerrisk <mtk.manpages@gmail.com>
> .\" and Copyright (c) 2017 David Howells <dhowells@redhat.com>
> .\"
> .\" %%%LICENSE_START(VERBATIM)
> .\" Permission is granted to make and distribute verbatim copies of this
> .\" manual provided the copyright notice and this permission notice are
> .\" preserved on all copies.
> .\"
> .\" Permission is granted to copy and distribute modified versions of this
> .\" manual under the conditions for verbatim copying, provided that the
> .\" entire resulting derived work is distributed under the terms of a
> .\" permission notice identical to this one.
> .\"
> .\" Since the Linux kernel and libraries are constantly changing, this
> .\" manual page may be incorrect or out-of-date.  The author(s) assume no
> .\" responsibility for errors or omissions, or for damages resulting from
> .\" the use of the information contained herein.  The author(s) may not
> .\" have taken the same level of care in the production of this manual,
> .\" which is licensed free of charge, as they might when working
> .\" professionally.
> .\"
> .\" Formatted or processed versions of this manual, if unaccompanied by
> .\" the source, must acknowledge the copyright and authors of this work.
> .\" %%%LICENSE_END
> .\"
> .TH STATX 2 2017-03-07 "Linux" "Linux Programmer's Manual"
> .SH NAME
> statx \- Get file status (extended)
> .SH SYNOPSIS
> .nf
> .B #include <sys/types.h>
> .br
> .B #include <sys/stat.h>
> .br
> .B #include <unistd.h>
> .br
> .BR "#include <fcntl.h>           " "/* Definition of AT_* constants */"
> .sp
> .BI "int statx(int " dirfd ", const char *" pathname ", int " flags ","
> .BI "          unsigned int " mask ", struct statx *" buf );
> .fi
> .sp
> .in -4n
> Feature Test Macro Requirements for glibc (see
> .BR feature_test_macros (7)):
> .in
> .ad l
> .PD 0
> .sp
> .RS 4
> <unknown as yet>
> .RE
> .PD
> .ad
> .SH DESCRIPTION
> .PP
> This function returns information about a file, storing it in the buffer
> pointed to by
> .IR buf .
> The buffer is filled in according to the following type:
> .PP
> .in +4n
> .nf
> struct statx {
>     __u32 stx_mask;          -- Mask of bits indicating filled fields
>     __u32 stx_blksize;       -- Block size for filesystem I/O
>     __u64 stx_attributes;    -- Extra file attribute indicators
>     __u32 stx_nlink;         -- Number of hard links
>     __u32 stx_uid;           -- User ID of owner
>     __u32 stx_gid;           -- Group ID of owner
>     __u16 stx_mode;          -- File type and mode
>     __u64 stx_ino;           -- Inode number
>     __u64 stx_size;          -- Total size in bytes
>     __u64 stx_blocks;        -- Number of 512B blocks allocated
>     struct statx_timestamp stx_atime;  -- Time of last access
>     struct statx_timestamp stx_btime;  -- Time of creation
>     struct statx_timestamp stx_ctime;  -- Time of last status change
>     struct statx_timestamp stx_mtime;  -- Time of last modification
>     __u32 stx_rdev_major;    } Device number if device file

"of device file" ?

>     __u32 stx_rdev_minor;    }
>     __u32 stx_dev_major;      } Device number of containing file
>     __u32 stx_dev_minor;      }

"Device number of device containing file"?

Or perhaps just "ID of device containing file" from the stat(2)
manpage?

> };
> .fi
> .in
> .PP
> Where the timestamps are defined as:
> .PP
> .in +4n
> .nf
> struct statx_timestamp {
>     __s64 tv_sec;    -- Number of seconds before or since 1970
>     __s32 tv_nsec;   -- Number of nanoseconds before or since tv_sec
> };
> .fi
> .in
> .PP
> (Note that reserved space and padding is ommitted)
> .SS
> Invoking \fBstatx\fR():
> .PP
> To access a file's status, no permissions are required on the file itself, but
> in the case of
> .BR statx ()
> with a path, execute (search) permission is required on all of the directories
> in
> .I pathname
> that lead to the file.
> .PP
> .BR statx ()
> uses
> .IR pathname ", " dirfd " and " flags
> to locate the target file in one of a variety of ways:
> .TP
> [*] By absolute path.
> .I pathname
> points to an absolute path and
> .I dirfd
> is ignored.  The file is looked up by name, starting from the root of the
> filesystem as seen by the calling process.
> .TP
> [*] By cwd-relative path.
> .I pathname
> points to a relative path and
> .IR dirfd " is " AT_FDCWD .
> The file is looked up by name, starting from the current working directory.
> .TP
> [*] By dir-relative path.
> .I pathname
> points to relative path and
> .I dirfd
> indicates a file descriptor pointing to a directory.  The file is looked up by
> name, starting from the directory specified by
> .IR dirfd .
> .TP
> [*] By file descriptor.
> .IR pathname " is " NULL " and " dirfd
> indicates a file descriptor.  The file attached to the file descriptor is
> queried directly.  The file descriptor may point to any type of file, not just
> a directory.
> .PP
> .I flags
> can be used to influence a path-based lookup.  A value for
> .I flags
> is constructed by OR'ing together zero or more of the following constants:
> .TP
> .BR AT_EMPTY_PATH " (since Linux 2.6.39)"
> .\" commit 65cfc6722361570bfe255698d9cd4dccaf47570d
> If
> .I pathname
> is an empty string, operate on the file referred to by
> .IR dirfd
> (which may have been obtained using the
> .BR open (2)
> .B O_PATH
> flag).
> If
> .I dirfd
> is
> .BR AT_FDCWD ,
> the call operates on the current working directory.
> In this case,
> .I dirfd
> can refer to any type of file, not just a directory.

Is (flags & AT_EMPTY_PATH) is the same as (pathname == NULL)?

> This flag is Linux-specific; define
> .B _GNU_SOURCE
> .\" Before glibc 2.16, defining _ATFILE_SOURCE sufficed
> to obtain its definition.
> .TP
> .BR AT_NO_AUTOMOUNT " (since Linux 2.6.38)"
> Don't automount the terminal ("basename") component of
> .I pathname
> if it is a directory that is an automount point.
> This allows the caller to gather attributes of an automount point
> (rather than the location it would mount).
> This flag can be used in tools that scan directories
> to prevent mass-automounting of a directory of automount points.
> The
> .B AT_NO_AUTOMOUNT
> flag has no effect if the mount point has already been mounted over.
> This flag is Linux-specific; define
> .B _GNU_SOURCE
> .\" Before glibc 2.16, defining _ATFILE_SOURCE sufficed
> to obtain its definition.
> .TP
> .B AT_SYMLINK_NOFOLLOW
> If
> .I pathname
> is a symbolic link, do not dereference it:
> instead return information about the link itself, like
> .BR lstat ().
> .PP
> .I flags
> can also be used to control what sort of synchronisation the kernel will do
> when querying a file on a remote filesystem.  This is done by OR'ing in one of
> the following values:
> .TP
> AT_STATX_SYNC_AS_STAT
> Do whatever
> .BR stat ()
> does.  This is the default and is very much filesystem specific.
> .TP
> AT_STATX_FORCE_SYNC
> Force the attributes to be synchronised with the server.  This may require that
> a network filesystem perform a data writeback to get the timestamps correct.
> .TP
> AT_STATX_DONT_SYNC
> Don't synchronise anything, but rather just take whatever the system has cached
> if possible.  This may mean that the information returned is approximate, but,
> on a network filesystem, it may not involve a round trip to the server - even
> if no lease is held.
> .PP
> The
> .I mask
> argument to
> .BR statx ()
> is used to tell the kernel which fields the caller is interested in
> .I mask
> is an OR'ed combination of the following constants:
> .PP
> .in +4n
> .TS
> lB l.
> STATX_TYPE	Want stx_mode & S_IFMT
> STATX_MODE	Want stx_mode & ~S_IFMT
> STATX_NLINK	Want stx_nlink
> STATX_UID	Want stx_uid
> STATX_GID	Want stx_gid
> STATX_ATIME	Want stx_atime{,_ns}
> STATX_MTIME	Want stx_mtime{,_ns}
> STATX_CTIME	Want stx_ctime{,_ns}
> STATX_INO	Want stx_ino
> STATX_SIZE	Want stx_size
> STATX_BLOCKS	Want stx_blocks
> STATX_BASIC_STATS	[The stuff in the normal stat struct]
> STATX_BTIME	Want stx_btime{,_ns}
> STATX_ALL	[All currently available stuff]
> .TE
> .in
> .PP
> .B "Do not"
> simply set
> .I mask
> to UINT_MAX as one or more bits may, in future, be used to specify an extension
> to the buffer.
> .SS
> The returned information
> .PP
> The status information for the target file is returned in the
> .I statx
> structure pointed to by
> .IR buf .
> Included in this is
> .I stx_mask
> which indicates what other information has been returned.
> .I stx_mask
> has the same format as the mask argument and bits are set in it to indicate
> which fields have been filled in.
> .PP
> It should be noted that the kernel may return fields that weren't requested and
> may fail to return fields that were requested, depending on what the backing
> filesystem supports.  In either case,
> .I stx_mask
> will not be equal
> .IR mask .
> .PP
> If a filesystem does not support a field or if it has an unrepresentable value
> (for instance, a file with an exotic type), then the mask bit corresponding to
> that field will be cleared in
> .I stx_mask
> even if the user asked for it and a dummy value will be filled in for
> compatibility purposes if one is available (e.g. a dummy uid and gid may be
> specified to mount under some circumstances).
> .PP
> A filesystem may also fill in fields that the caller didn't ask for if it has
> values for them available at no extra cost.  If this happens, the corresponding
> bits will be set in
> .IR stx_mask .
> .PP
> 
> .\" Background: inode attributes are modified with i_mutex held, but
> .\" read by stat() without taking the mutex.
> .I Note:
> For performance and simplicity reasons, different fields in the
> .I statx
> structure may contain state information from different moments
> during the execution of the system call.

Hm.  Judging from the ext4 patch you proposed, I gather this is
expected, at least in the btime case.

--D

> For example, if
> .IR stx_mode
> or
> .IR stx_uid
> is changed by another process by calling
> .BR chmod (2)
> or
> .BR chown (2),
> .BR stat ()
> might return the old
> .I stx_mode
> together with the new
> .IR stx_uid ,
> or the old
> .I stx_uid
> together with the new
> .IR stx_mode .
> .PP
> Apart from stx_mask (which is described above), the fields in the
> .I statx
> structure are:
> .TP
> .I stx_mode
> The file type and mode.  This is described in more detail below.
> .TP
> .I stx_size
> The size of the file (if it is a regular file or a symbolic link) in bytes.
> The size of a symbolic link is the length of the pathname it contains, without
> a terminating null byte.
> .TP
> .I stx_blocks
> The number of blocks allocated to the file on the medium, in 512-byte units.
> (This may be smaller than
> .IR stx_size /512
> when the file has holes.)
> .TP
> .I stx_blksize
> The "preferred" blocksize for efficient filesystem I/O.  (Writing to a file in
> smaller chunks may cause an inefficient read-modify-rewrite.)
> .TP
> .I stx_nlink
> The number of hard links on a file.
> .TP
> .I stx_uid
> The user ID of the file's owner.
> .TP
> .I stx_gid
> The ID of the group that may access the file.
> .TP
> .IR stx_dev_major " and "  stx_dev_minor
> The device on which this file (inode) resides.
> .TP
> .IR stx_rdev_major " and "  stx_rdev_minor
> The device that this file (inode) represents if the file is of block or
> character device type.
> .TP
> .I stx_attributes
> Further status information about the file.  This consists of zero or more of
> the following constants OR'ed together:
> .in +4n
> .TS
> lB l.
> STATX_ATTR_COMPRESSED	File is compressed by the fs
> STATX_ATTR_IMMUTABLE	File is marked immutable
> STATX_ATTR_APPEND	File is append-only
> STATX_ATTR_NODUMP	File is not to be dumped
> STATX_ATTR_ENCRYPTED	File requires key to decrypt in fs
> .TE
> .in
> .TP
> .I stx_atime
> The file's last access timestamp.
> This field is changed by file accesses, for example, by
> .BR execve (2),
> .BR mknod (2),
> .BR pipe (2),
> .BR utime (2),
> and
> .BR read (2)
> (of more than zero bytes).
> Other routines, such as
> .BR mmap (2),
> may or may not update it.
> .TP
> .I stx_btime
> The file's creation timestamp.  This is set on file creation and not changed
> subsequently.
> .TP
> .I stx_ctime
> The file's last status change timestamp.  This field is changed by writing or
> by setting inode information (i.e., owner, group, link count, mode, etc.).
> .TP
> .I stx_mtime
> The file's last modification timestamp.  This is changed by file modifications,
> for example, by
> .BR mknod (2),
> .BR truncate (2),
> .BR utime (2),
> and
> .BR write (2)
> (of more than zero bytes).  Moreover, the modification time of a directory is
> changed by the creation or deletion of files in that directory.  This field is
> .I not
> changed for changes in owner, group, hard link count, or mode.
> 
> 
> 
> .PP
> Not all of the Linux filesystems implement all of the timestamp fields.  Some
> filesystems allow mounting in such a way that file and/or directory accesses do
> not cause an update of the
> .I stx_atime
> field.
> (See
> .IR noatime ,
> .IR nodiratime ,
> and
> .I relatime
> in
> .BR mount (8),
> and related information in
> .BR mount (2).)
> In addition,
> .I stx_atime
> is not updated if a file is opened with the
> .BR O_NOATIME ;
> see
> .BR open (2).
> 
> .SS File type and mode
> .PP
> The
> .I stx_mode
> field contains the combined file type and mode.  POSIX refers to the bits in
> this field corresponding to the mask
> .B S_IFMT
> (see below) as the
> .IR "file type" ,
> the 12 bits corresponding to the mask 07777 as the
> .IR "file mode bits"
> and the least significant 9 bits (0777) as the
> .IR "file permission bits" .
> .IP
> The following mask values are defined for the file type of the
> .I stx_mode
> field:
> .in +4n
> .TS
> lB l l.
> S_IFMT	0170000	bit mask for the file type bit field
> 
> S_IFSOCK	0140000	socket
> S_IFLNK	0120000	symbolic link
> S_IFREG	0100000	regular file
> S_IFBLK	0060000	block device
> S_IFDIR	0040000	directory
> S_IFCHR	0020000	character device
> S_IFIFO	0010000	FIFO
> .TE
> .in
> .IP
> Note that
> .I stx_mode
> has two mask flags covering it: one for the type and one for the mode bits.
> .PP
> To test for a regular file (for example), one could write:
> .nf
> .in +4n
> statx(AT_FDCWD, pathname, 0, STATX_BASIC_STATS, &sb);
> if ((sb.stx_mode & S_IFMT) == S_IFREG) {
>     /* Handle regular file */
> }
> .in
> .fi
> .PP
> Because tests of the above form are common, additional macros are defined by
> POSIX to allow the test of the file type in
> .I stx_mode
> to be written more concisely:
> .RS 4
> .TS
> lB l.
> \fBS_ISREG\fR(m)	Is it a regular file?
> \fBS_ISDIR\fR(m)	Is it a directory?
> \fBS_ISCHR\fR(m)	Is it a character device?
> \fBS_ISBLK\fR(m)	Is it a block device?
> \fBS_ISFIFO\fR(m)	Is it a FIFO (named pipe)?
> \fBS_ISLNK\fR(m)	Is it a symbolic link?  (Not in POSIX.1-1996.)
> \fBS_ISSOCK\fR(m)	Is it a socket?  (Not in POSIX.1-1996.)
> .TE
> .RE
> .PP
> The preceding code snippet could thus be rewritten as:
> 
> .nf
> .in +4n
> statx(AT_FDCWD, pathname, 0, STATX_BASIC_STATS, &sb);
> if (S_ISREG(sb.stx_mode)) {
>     /* Handle regular file */
> }
> .in
> .fi
> .PP
> The definitions of most of the above file type test macros
> are provided if any of the following feature test macros is defined:
> .BR _BSD_SOURCE
> (in glibc 2.19 and earlier),
> .BR _SVID_SOURCE
> (in glibc 2.19 and earlier),
> or
> .BR _DEFAULT_SOURCE
> (in glibc 2.20 and later).
> In addition, definitions of all of the above macros except
> .BR S_IFSOCK
> and
> .BR S_ISSOCK ()
> are provided if
> .BR _XOPEN_SOURCE
> is defined.
> The definition of
> .BR S_IFSOCK
> can also be exposed by defining
> .BR _XOPEN_SOURCE
> with a value of 500 or greater.
> 
> The definition of
> .BR S_ISSOCK ()
> is exposed if any of the following feature test macros is defined:
> .BR _BSD_SOURCE
> (in glibc 2.19 and earlier),
> .BR _DEFAULT_SOURCE
> (in glibc 2.20 and later),
> .BR _XOPEN_SOURCE
> with a value of 500 or greater, or
> .BR _POSIX_C_SOURCE
> with a value of 200112L or greater.
> .PP
> The following mask values are defined for
> the file mode component of the
> .I stx_mode
> field:
> .in +4n
> .TS
> lB l l.
> S_ISUID	  04000	set-user-ID bit
> S_ISGID	  02000	set-group-ID bit (see below)
> S_ISVTX	  01000	sticky bit (see below)
> 
> S_IRWXU	  00700	owner has read, write, and execute permission
> S_IRUSR	  00400	owner has read permission
> S_IWUSR	  00200	owner has write permission
> S_IXUSR	  00100	owner has execute permission
> 
> S_IRWXG	  00070	group has read, write, and execute permission
> S_IRGRP	  00040	group has read permission
> S_IWGRP	  00020	group has write permission
> S_IXGRP	  00010	group has execute permission
> 
> S_IRWXO	  00007	T{
> others (not in group) have read, write, and execute permission
> T}
> S_IROTH	  00004	others have read permission
> S_IWOTH	  00002	others have write permission
> S_IXOTH	  00001	others have execute permission
> .TE
> .in
> .P
> The set-group-ID bit
> .RB ( S_ISGID )
> has several special uses.
> For a directory, it indicates that BSD semantics is to be used
> for that directory: files created there inherit their group ID from
> the directory, not from the effective group ID of the creating process,
> and directories created there will also get the
> .B S_ISGID
> bit set.
> For a file that does not have the group execution bit
> .RB ( S_IXGRP )
> set,
> the set-group-ID bit indicates mandatory file/record locking.
> .P
> The sticky bit
> .RB ( S_ISVTX )
> on a directory means that a file
> in that directory can be renamed or deleted only by the owner
> of the file, by the owner of the directory, and by a privileged
> process.
> 
> 
> .SH RETURN VALUE
> On success, zero is returned.
> On error, \-1 is returned, and
> .I errno
> is set appropriately.
> .SH ERRORS
> .TP
> .B EINVAL
> Invalid flag specified in
> .IR flags .
> .TP
> .B EACCES
> Search permission is denied for one of the directories
> in the path prefix of
> .IR pathname .
> (See also
> .BR path_resolution (7).)
> .TP
> .B EBADF
> .I dirfd
> is not a valid open file descriptor.
> .TP
> .B EFAULT
> Bad address.
> .TP
> .B ELOOP
> Too many symbolic links encountered while traversing the path.
> .TP
> .B ENAMETOOLONG
> .I pathname
> is too long.
> .TP
> .B ENOENT
> A component of
> .I pathname
> does not exist, or
> .I pathname
> is an empty string.
> .TP
> .B ENOMEM
> Out of memory (i.e., kernel memory).
> .TP
> .B ENOTDIR
> A component of the path prefix of
> .I pathname
> is not a directory or
> .I pathname
> is relative and
> .I dirfd
> is a file descriptor referring to a file other than a directory.
> .SH VERSIONS
> .BR statx ()
> was added to Linux in kernel 4.11;
> library support is not yet added to glibc.
> .SH SEE ALSO
> .BR ls (1),
> .BR stat (1),
> .BR access (2),
> .BR chmod (2),
> .BR chown (2),
> .BR readlink (2),
> .BR utime (2),
> .BR capabilities (7),
> .BR symlink (7)

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
  2017-03-07 18:02   ` Darrick J. Wong
@ 2017-03-07 18:39   ` David Howells
  2017-03-07 18:44     ` Darrick J. Wong
  2017-03-07 18:55     ` David Howells
  2017-03-07 21:44   ` Andreas Dilger
                     ` (9 subsequent siblings)
  11 siblings, 2 replies; 38+ messages in thread
From: David Howells @ 2017-03-07 18:39 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: dhowells, Christoph Hellwig, mtk.manpages, linux-fsdevel, xfs

Darrick J. Wong <darrick.wong@oracle.com> wrote:

> Is (flags & AT_EMPTY_PATH) is the same as (pathname == NULL)?

I'm not sure.  I was wondering about that whilst writing this.  The text
describing the option is copied from the stat(2) manpage.  It specified an
empty path (presumably "") rather than NULL.  I'm also not sure whether it has
other ramifications.

> > .I Note:
> > For performance and simplicity reasons, different fields in the
> > .I statx
> > structure may contain state information from different moments
> > during the execution of the system call.
> 
> Hm.  Judging from the ext4 patch you proposed, I gather this is
> expected, at least in the btime case.

This is copied from the stat(2) manpage.  The comment in the source there says
it's because there's no locking in the getattr path against the setattr path.

David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 18:39   ` David Howells
@ 2017-03-07 18:44     ` Darrick J. Wong
  2017-03-07 18:55     ` David Howells
  1 sibling, 0 replies; 38+ messages in thread
From: Darrick J. Wong @ 2017-03-07 18:44 UTC (permalink / raw)
  To: David Howells; +Cc: Christoph Hellwig, mtk.manpages, linux-fsdevel, xfs

On Tue, Mar 07, 2017 at 06:39:42PM +0000, David Howells wrote:
> Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> > Is (flags & AT_EMPTY_PATH) is the same as (pathname == NULL)?
> 
> I'm not sure.  I was wondering about that whilst writing this.  The text
> describing the option is copied from the stat(2) manpage.  It specified an
> empty path (presumably "") rather than NULL.  I'm also not sure whether it has
> other ramifications.

<nod>

> > > .I Note:
> > > For performance and simplicity reasons, different fields in the
> > > .I statx
> > > structure may contain state information from different moments
> > > during the execution of the system call.
> > 
> > Hm.  Judging from the ext4 patch you proposed, I gather this is
> > expected, at least in the btime case.

Ugh, I put that in the wrong part -- this was supposed to be a comment
about the part where the manpage states that filesystems can return more
than userspace asked for.  My eyes are bad at reading manpage source. :/

(Sorry for the noise.)

--D

> This is copied from the stat(2) manpage.  The comment in the source there says
> it's because there's no locking in the getattr path against the setattr path.
> 
> David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 18:39   ` David Howells
  2017-03-07 18:44     ` Darrick J. Wong
@ 2017-03-07 18:55     ` David Howells
  1 sibling, 0 replies; 38+ messages in thread
From: David Howells @ 2017-03-07 18:55 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: dhowells, Christoph Hellwig, mtk.manpages, linux-fsdevel, xfs

Darrick J. Wong <darrick.wong@oracle.com> wrote:

> > > Hm.  Judging from the ext4 patch you proposed, I gather this is
> > > expected, at least in the btime case.
> 
> Ugh, I put that in the wrong part -- this was supposed to be a comment
> about the part where the manpage states that filesystems can return more
> than userspace asked for.

In that case, sort of.  You could just ask for STATX_TYPE | STATX_MODE, for
example, but the kernel is perfectly at liberty to add STATX_MTIME,
STATX_SIZE, STATX_INO, etc. even if it doesn't add STATX_BTIME as well.

> My eyes are bad at reading manpage source. :/

I know what you mean.

David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
  2017-03-07 18:02   ` Darrick J. Wong
  2017-03-07 18:39   ` David Howells
@ 2017-03-07 21:44   ` Andreas Dilger
  2017-03-07 22:55   ` Eric Biggers
                     ` (8 subsequent siblings)
  11 siblings, 0 replies; 38+ messages in thread
From: Andreas Dilger @ 2017-03-07 21:44 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, Michael Kerrisk (man-pages),
	Darrick J. Wong, linux-fsdevel, xfs

[-- Attachment #1: Type: text/plain, Size: 21271 bytes --]


> On Mar 7, 2017, at 10:22 AM, David Howells <dhowells@redhat.com> wrote:
> 
> Christoph Hellwig <hch@infradead.org> wrote:
> 
>> This would be great to have in 4.11 together with the initial statx
>> implementation.  But until I see documentation and testcases for statx
>> I don't really feel comfortable reviewing anything related to it.
> 
> Well, since you asked for documentation, here's a manual page for you to
> review:-)
> 
> Note that as it isn't in glibc yet, I've left out all the
> set-this-and-that-#define-to-make-it-appear stuff except where it is pertinent
> to particular constants.
> 
> I don't suppose you know where the documentation on writing xfstests tests is?
> xfstests-dev/doc/ only contains an old and out of date changelog.
> 
> David
> ---
> '\" t
> .\" Copyright (c) 1992 Drew Eckhardt (drew@cs.colorado.edu), March 28, 1992
> .\" Parts Copyright (c) 1995 Nicolai Langfeldt (janl@ifi.uio.no), 1/1/95
> .\" and Copyright (c) 2006, 2007, 2014 Michael Kerrisk <mtk.manpages@gmail.com>
> .\" and Copyright (c) 2017 David Howells <dhowells@redhat.com>
> .\"
> .\" %%%LICENSE_START(VERBATIM)
> .\" Permission is granted to make and distribute verbatim copies of this
> .\" manual provided the copyright notice and this permission notice are
> .\" preserved on all copies.
> .\"
> .\" Permission is granted to copy and distribute modified versions of this
> .\" manual under the conditions for verbatim copying, provided that the
> .\" entire resulting derived work is distributed under the terms of a
> .\" permission notice identical to this one.
> .\"
> .\" Since the Linux kernel and libraries are constantly changing, this
> .\" manual page may be incorrect or out-of-date.  The author(s) assume no
> .\" responsibility for errors or omissions, or for damages resulting from
> .\" the use of the information contained herein.  The author(s) may not
> .\" have taken the same level of care in the production of this manual,
> .\" which is licensed free of charge, as they might when working
> .\" professionally.
> .\"
> .\" Formatted or processed versions of this manual, if unaccompanied by
> .\" the source, must acknowledge the copyright and authors of this work.
> .\" %%%LICENSE_END
> .\"
> .TH STATX 2 2017-03-07 "Linux" "Linux Programmer's Manual"
> .SH NAME
> statx \- Get file status (extended)
> .SH SYNOPSIS
> .nf
> .B #include <sys/types.h>
> .br
> .B #include <sys/stat.h>
> .br
> .B #include <unistd.h>
> .br
> .BR "#include <fcntl.h>           " "/* Definition of AT_* constants */"
> .sp
> .BI "int statx(int " dirfd ", const char *" pathname ", int " flags ","
> .BI "          unsigned int " mask ", struct statx *" buf );
> .fi
> .sp
> .in -4n
> Feature Test Macro Requirements for glibc (see
> .BR feature_test_macros (7)):
> .in
> .ad l
> .PD 0
> .sp
> .RS 4
> <unknown as yet>
> .RE
> .PD
> .ad
> .SH DESCRIPTION
> .PP
> This function returns information about a file, storing it in the buffer
> pointed to by
> .IR buf .
> The buffer is filled in according to the following type:
> .PP
> .in +4n
> .nf
> struct statx {
>    __u32 stx_mask;          -- Mask of bits indicating filled fields
>    __u32 stx_blksize;       -- Block size for filesystem I/O
>    __u64 stx_attributes;    -- Extra file attribute indicators
>    __u32 stx_nlink;         -- Number of hard links
>    __u32 stx_uid;           -- User ID of owner
>    __u32 stx_gid;           -- Group ID of owner
>    __u16 stx_mode;          -- File type and mode
>    __u64 stx_ino;           -- Inode number
>    __u64 stx_size;          -- Total size in bytes
>    __u64 stx_blocks;        -- Number of 512B blocks allocated
>    struct statx_timestamp stx_atime;  -- Time of last access
>    struct statx_timestamp stx_btime;  -- Time of creation
>    struct statx_timestamp stx_ctime;  -- Time of last status change
>    struct statx_timestamp stx_mtime;  -- Time of last modification
>    __u32 stx_rdev_major;    } Device number if device file
>    __u32 stx_rdev_minor;    }
>    __u32 stx_dev_major;      } Device number of containing file
>    __u32 stx_dev_minor;      }
> };
> .fi
> .in
> .PP
> Where the timestamps are defined as:
> .PP
> .in +4n
> .nf
> struct statx_timestamp {
>    __s64 tv_sec;    -- Number of seconds before or since 1970
>    __s32 tv_nsec;   -- Number of nanoseconds before or since tv_sec
> };
> .fi
> .in
> .PP
> (Note that reserved space and padding is ommitted)

Do you think that not including the padding could be problematic for users?

> .SS
> Invoking \fBstatx\fR():
> .PP
> To access a file's status, no permissions are required on the file itself, but
> in the case of
> .BR statx ()
> with a path, execute (search) permission is required on all of the directories
> in
> .I pathname
> that lead to the file.
> .PP
> .BR statx ()
> uses
> .IR pathname ", " dirfd " and " flags
> to locate the target file in one of a variety of ways:
> .TP
> [*] By absolute path.
> .I pathname
> points to an absolute path and
> .I dirfd
> is ignored.  The file is looked up by name, starting from the root of the
> filesystem as seen by the calling process.
> .TP
> [*] By cwd-relative path.
> .I pathname
> points to a relative path and
> .IR dirfd " is " AT_FDCWD .
> The file is looked up by name, starting from the current working directory.
> .TP
> [*] By dir-relative path.
> .I pathname
> points to relative path and
> .I dirfd
> indicates a file descriptor pointing to a directory.  The file is looked up by
> name, starting from the directory specified by
> .IR dirfd .
> .TP
> [*] By file descriptor.
> .IR pathname " is " NULL " and " dirfd
> indicates a file descriptor.  The file attached to the file descriptor is
> queried directly.  The file descriptor may point to any type of file, not just
> a directory.
> .PP
> .I flags
> can be used to influence a path-based lookup.  A value for
> .I flags
> is constructed by OR'ing together zero or more of the following constants:
> .TP
> .BR AT_EMPTY_PATH " (since Linux 2.6.39)"
> .\" commit 65cfc6722361570bfe255698d9cd4dccaf47570d
> If
> .I pathname
> is an empty string, operate on the file referred to by
> .IR dirfd
> (which may have been obtained using the
> .BR open (2)
> .B O_PATH
> flag).
> If
> .I dirfd
> is
> .BR AT_FDCWD ,
> the call operates on the current working directory.
> In this case,
> .I dirfd
> can refer to any type of file, not just a directory.
> This flag is Linux-specific; define
> .B _GNU_SOURCE
> .\" Before glibc 2.16, defining _ATFILE_SOURCE sufficed
> to obtain its definition.
> .TP
> .BR AT_NO_AUTOMOUNT " (since Linux 2.6.38)"
> Don't automount the terminal ("basename") component of
> .I pathname
> if it is a directory that is an automount point.
> This allows the caller to gather attributes of an automount point
> (rather than the location it would mount).
> This flag can be used in tools that scan directories
> to prevent mass-automounting of a directory of automount points.
> The
> .B AT_NO_AUTOMOUNT
> flag has no effect if the mount point has already been mounted over.
> This flag is Linux-specific; define
> .B _GNU_SOURCE
> .\" Before glibc 2.16, defining _ATFILE_SOURCE sufficed
> to obtain its definition.
> .TP
> .B AT_SYMLINK_NOFOLLOW
> If
> .I pathname
> is a symbolic link, do not dereference it:
> instead return information about the link itself, like
> .BR lstat ().
> .PP
> .I flags
> can also be used to control what sort of synchronisation the kernel will do
> when querying a file on a remote filesystem.  This is done by OR'ing in one of
> the following values:
> .TP
> AT_STATX_SYNC_AS_STAT
> Do whatever
> .BR stat ()
> does.  This is the default and is very much filesystem specific.
> .TP
> AT_STATX_FORCE_SYNC
> Force the attributes to be synchronised with the server.  This may require that
> a network filesystem perform a data writeback to get the timestamps correct.
> .TP
> AT_STATX_DONT_SYNC
> Don't synchronise anything, but rather just take whatever the system has cached
> if possible.  This may mean that the information returned is approximate, but,
> on a network filesystem, it may not involve a round trip to the server - even
> if no lease is held.
> .PP
> The
> .I mask
> argument to
> .BR statx ()
> is used to tell the kernel which fields the caller is interested in
> .I mask
> is an OR'ed combination of the following constants:
> .PP
> .in +4n
> .TS
> lB l.
> STATX_TYPE	Want stx_mode & S_IFMT
> STATX_MODE	Want stx_mode & ~S_IFMT
> STATX_NLINK	Want stx_nlink
> STATX_UID	Want stx_uid
> STATX_GID	Want stx_gid
> STATX_ATIME	Want stx_atime{,_ns}
> STATX_MTIME	Want stx_mtime{,_ns}
> STATX_CTIME	Want stx_ctime{,_ns}
> STATX_INO	Want stx_ino
> STATX_SIZE	Want stx_size
> STATX_BLOCKS	Want stx_blocks
> STATX_BASIC_STATS	[The stuff in the normal stat struct]
> STATX_BTIME	Want stx_btime{,_ns}
> STATX_ALL	[All currently available stuff]
> .TE
> .in
> .PP
> .B "Do not"
> simply set
> .I mask
> to UINT_MAX as one or more bits may, in future, be used to specify an extension
> to the buffer.
> .SS
> The returned information
> .PP
> The status information for the target file is returned in the
> .I statx
> structure pointed to by
> .IR buf .
> Included in this is
> .I stx_mask
> which indicates what other information has been returned.
> .I stx_mask
> has the same format as the mask argument and bits are set in it to indicate
> which fields have been filled in.
> .PP
> It should be noted that the kernel may return fields that weren't requested and
> may fail to return fields that were requested, depending on what the backing
> filesystem supports.  In either case,
> .I stx_mask
> will not be equal
> .IR mask .
> .PP
> If a filesystem does not support a field or if it has an unrepresentable value
> (for instance, a file with an exotic type), then the mask bit corresponding to
> that field will be cleared in
> .I stx_mask
> even if the user asked for it and a dummy value will be filled in for
> compatibility purposes if one is available (e.g. a dummy uid and gid may be
> specified to mount under some circumstances).
> .PP
> A filesystem may also fill in fields that the caller didn't ask for if it has
> values for them available at no extra cost.  If this happens, the corresponding
> bits will be set in
> .IR stx_mask .
> .PP
> 
> .\" Background: inode attributes are modified with i_mutex held, but
> .\" read by stat() without taking the mutex.
> .I Note:
> For performance and simplicity reasons, different fields in the
> .I statx
> structure may contain state information from different moments
> during the execution of the system call.
> For example, if
> .IR stx_mode
> or
> .IR stx_uid
> is changed by another process by calling
> .BR chmod (2)
> or
> .BR chown (2),
> .BR stat ()
> might return the old
> .I stx_mode
> together with the new
> .IR stx_uid ,
> or the old
> .I stx_uid
> together with the new
> .IR stx_mode .
> .PP
> Apart from stx_mask (which is described above), the fields in the
> .I statx
> structure are:
> .TP
> .I stx_mode
> The file type and mode.  This is described in more detail below.
> .TP
> .I stx_size
> The size of the file (if it is a regular file or a symbolic link) in bytes.
> The size of a symbolic link is the length of the pathname it contains, without
> a terminating null byte.
> .TP
> .I stx_blocks
> The number of blocks allocated to the file on the medium, in 512-byte units.
> (This may be smaller than
> .IR stx_size /512
> when the file has holes.)
> .TP
> .I stx_blksize
> The "preferred" blocksize for efficient filesystem I/O.  (Writing to a file in
> smaller chunks may cause an inefficient read-modify-rewrite.)
> .TP
> .I stx_nlink
> The number of hard links on a file.
> .TP
> .I stx_uid
> The user ID of the file's owner.
> .TP
> .I stx_gid
> The ID of the group that may access the file.
> .TP
> .IR stx_dev_major " and "  stx_dev_minor
> The device on which this file (inode) resides.
> .TP
> .IR stx_rdev_major " and "  stx_rdev_minor
> The device that this file (inode) represents if the file is of block or
> character device type.
> .TP
> .I stx_attributes
> Further status information about the file.  This consists of zero or more of
> the following constants OR'ed together:
> .in +4n
> .TS
> lB l.
> STATX_ATTR_COMPRESSED	File is compressed by the fs
> STATX_ATTR_IMMUTABLE	File is marked immutable

This definition is circular...  Maybe "File cannot be changed once closed"?

> STATX_ATTR_APPEND	File is append-only

This is also circular.  Maybe "file can only be written after .B stx_size"?

> STATX_ATTR_NODUMP	File is not to be dumped

This one is also circular.  Maybe "not to be included in backups"?

> STATX_ATTR_ENCRYPTED	File requires key to decrypt in fs
> .TE
> .in
> .TP
> .I stx_atime
> The file's last access timestamp.
> This field is changed by file accesses, for example, by
> .BR execve (2),
> .BR mknod (2),
> .BR pipe (2),
> .BR utime (2),
> and
> .BR read (2)
> (of more than zero bytes).
> Other routines, such as
> .BR mmap (2),
> may or may not update it.
> .TP
> .I stx_btime
> The file's creation timestamp.  This is set on file creation and not changed
> subsequently.
> .TP
> .I stx_ctime
> The file's last status change timestamp.  This field is changed by writing or
> by setting inode information (i.e., owner, group, link count, mode, etc.).
> .TP
> .I stx_mtime
> The file's last modification timestamp.  This is changed by file modifications,
> for example, by
> .BR mknod (2),
> .BR truncate (2),
> .BR utime (2),
> and
> .BR write (2)
> (of more than zero bytes).  Moreover, the modification time of a directory is
> changed by the creation or deletion of files in that directory.  This field is
> .I not
> changed for changes in owner, group, hard link count, or mode.
> 
> 
> 
> .PP
> Not all of the Linux filesystems implement all of the timestamp fields.  Some
> filesystems allow mounting in such a way that file and/or directory accesses do
> not cause an update of the
> .I stx_atime
> field.
> (See
> .IR noatime ,
> .IR nodiratime ,
> and
> .I relatime
> in
> .BR mount (8),
> and related information in
> .BR mount (2).)
> In addition,
> .I stx_atime
> is not updated if a file is opened with the
> .BR O_NOATIME ;
> see
> .BR open (2).
> 
> .SS File type and mode
> .PP
> The
> .I stx_mode
> field contains the combined file type and mode.  POSIX refers to the bits in
> this field corresponding to the mask
> .B S_IFMT
> (see below) as the
> .IR "file type" ,
> the 12 bits corresponding to the mask 07777 as the
> .IR "file mode bits"
> and the least significant 9 bits (0777) as the
> .IR "file permission bits" .
> .IP
> The following mask values are defined for the file type of the
> .I stx_mode
> field:
> .in +4n
> .TS
> lB l l.
> S_IFMT	0170000	bit mask for the file type bit field
> 
> S_IFSOCK	0140000	socket
> S_IFLNK	0120000	symbolic link
> S_IFREG	0100000	regular file
> S_IFBLK	0060000	block device
> S_IFDIR	0040000	directory
> S_IFCHR	0020000	character device
> S_IFIFO	0010000	FIFO
> .TE
> .in
> .IP
> Note that
> .I stx_mode
> has two mask flags covering it: one for the type and one for the mode bits.
> .PP
> To test for a regular file (for example), one could write:
> .nf
> .in +4n
> statx(AT_FDCWD, pathname, 0, STATX_BASIC_STATS, &sb);
> if ((sb.stx_mode & S_IFMT) == S_IFREG) {
>    /* Handle regular file */
> }

Two notes on these two examples:
- users unfamiliar with statx() may benefit from seeing STATX_TYPE used
  here instead of STATX_BASIC_STATS
- the check should also look for the presence of STATX_TYPE in the
  returned stx_mask to ensure it is valid before use?


   To test whether a path is a regular file (for example), one could write:
   .nf
   .in +4n
   rc = statx(AT_FDCWD, pathname, 0, STATX_TYPE, &stx);
   if (stx.stx_mask & STATX_TYPE && S_ISREG(sb.stx_mode)) {
      /* Handle regular file */
   }



> Because tests of the above form are common, additional macros are defined by
> POSIX to allow the test of the file type in
> .I stx_mode
> to be written more concisely:

Should this all just reference the existing stat(2) man page instead of
duplicating the whole contents here?  This is spending a lot of space
discussing the stx_mode field which could be avoided.

> .RS 4
> .TS
> lB l.
> \fBS_ISREG\fR(m)	Is it a regular file?
> \fBS_ISDIR\fR(m)	Is it a directory?
> \fBS_ISCHR\fR(m)	Is it a character device?
> \fBS_ISBLK\fR(m)	Is it a block device?
> \fBS_ISFIFO\fR(m)	Is it a FIFO (named pipe)?
> \fBS_ISLNK\fR(m)	Is it a symbolic link?  (Not in POSIX.1-1996.)
> \fBS_ISSOCK\fR(m)	Is it a socket?  (Not in POSIX.1-1996.)
> .TE
> .RE
> .PP
> The preceding code snippet could thus be rewritten as:
> 
> .nf
> .in +4n
> statx(AT_FDCWD, pathname, 0, STATX_BASIC_STATS, &sb);
> if (S_ISREG(sb.stx_mode)) {
>    /* Handle regular file */
> }
> .in
> .fi
> .PP
> The definitions of most of the above file type test macros
> are provided if any of the following feature test macros is defined:
> .BR _BSD_SOURCE
> (in glibc 2.19 and earlier),
> .BR _SVID_SOURCE
> (in glibc 2.19 and earlier),
> or
> .BR _DEFAULT_SOURCE
> (in glibc 2.20 and later).
> In addition, definitions of all of the above macros except
> .BR S_IFSOCK
> and
> .BR S_ISSOCK ()
> are provided if
> .BR _XOPEN_SOURCE
> is defined.
> The definition of
> .BR S_IFSOCK
> can also be exposed by defining
> .BR _XOPEN_SOURCE
> with a value of 500 or greater.
> 
> The definition of
> .BR S_ISSOCK ()
> is exposed if any of the following feature test macros is defined:
> .BR _BSD_SOURCE
> (in glibc 2.19 and earlier),
> .BR _DEFAULT_SOURCE
> (in glibc 2.20 and later),
> .BR _XOPEN_SOURCE
> with a value of 500 or greater, or
> .BR _POSIX_C_SOURCE
> with a value of 200112L or greater.
> .PP
> The following mask values are defined for
> the file mode component of the
> .I stx_mode
> field:
> .in +4n
> .TS
> lB l l.
> S_ISUID	  04000	set-user-ID bit
> S_ISGID	  02000	set-group-ID bit (see below)
> S_ISVTX	  01000	sticky bit (see below)
> 
> S_IRWXU	  00700	owner has read, write, and execute permission
> S_IRUSR	  00400	owner has read permission
> S_IWUSR	  00200	owner has write permission
> S_IXUSR	  00100	owner has execute permission
> 
> S_IRWXG	  00070	group has read, write, and execute permission
> S_IRGRP	  00040	group has read permission
> S_IWGRP	  00020	group has write permission
> S_IXGRP	  00010	group has execute permission
> 
> S_IRWXO	  00007	T{
> others (not in group) have read, write, and execute permission
> T}
> S_IROTH	  00004	others have read permission
> S_IWOTH	  00002	others have write permission
> S_IXOTH	  00001	others have execute permission
> .TE
> .in
> .P
> The set-group-ID bit
> .RB ( S_ISGID )
> has several special uses.
> For a directory, it indicates that BSD semantics is to be used
> for that directory: files created there inherit their group ID from
> the directory, not from the effective group ID of the creating process,
> and directories created there will also get the
> .B S_ISGID
> bit set.
> For a file that does not have the group execution bit
> .RB ( S_IXGRP )
> set,
> the set-group-ID bit indicates mandatory file/record locking.
> .P
> The sticky bit
> .RB ( S_ISVTX )
> on a directory means that a file
> in that directory can be renamed or deleted only by the owner
> of the file, by the owner of the directory, and by a privileged
> process.
> 
> 
> .SH RETURN VALUE
> On success, zero is returned.
> On error, \-1 is returned, and
> .I errno
> is set appropriately.
> .SH ERRORS
> .TP
> .B EINVAL
> Invalid flag specified in
> .IR flags .
> .TP
> .B EACCES
> Search permission is denied for one of the directories
> in the path prefix of
> .IR pathname .
> (See also
> .BR path_resolution (7).)
> .TP
> .B EBADF
> .I dirfd
> is not a valid open file descriptor.
> .TP
> .B EFAULT
> Bad address.
> .TP
> .B ELOOP
> Too many symbolic links encountered while traversing the path.
> .TP
> .B ENAMETOOLONG
> .I pathname
> is too long.
> .TP
> .B ENOENT
> A component of
> .I pathname
> does not exist, or
> .I pathname
> is an empty string.
> .TP
> .B ENOMEM
> Out of memory (i.e., kernel memory).
> .TP
> .B ENOTDIR
> A component of the path prefix of
> .I pathname
> is not a directory or
> .I pathname
> is relative and
> .I dirfd
> is a file descriptor referring to a file other than a directory.
> .SH VERSIONS
> .BR statx ()
> was added to Linux in kernel 4.11;
> library support is not yet added to glibc.
> .SH SEE ALSO
> .BR ls (1),
> .BR stat (1),

Maybe stat(2) ? :-)

> .BR access (2),
> .BR chmod (2),
> .BR chown (2),
> .BR readlink (2),
> .BR utime (2),
> .BR capabilities (7),
> .BR symlink (7)


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
                     ` (2 preceding siblings ...)
  2017-03-07 21:44   ` Andreas Dilger
@ 2017-03-07 22:55   ` Eric Biggers
  2017-03-08  3:45   ` Eryu Guan
                     ` (7 subsequent siblings)
  11 siblings, 0 replies; 38+ messages in thread
From: Eric Biggers @ 2017-03-07 22:55 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, mtk.manpages, Darrick J. Wong, linux-fsdevel, xfs

On Tue, Mar 07, 2017 at 05:22:55PM +0000, David Howells wrote:
> .fi
> .in
> .PP
> (Note that reserved space and padding is ommitted)

ommitted => omitted

> .TP
> .BR AT_EMPTY_PATH " (since Linux 2.6.39)"
...
> .TP
> .BR AT_NO_AUTOMOUNT " (since Linux 2.6.38)"

Given that statx is just being added now in 4.11, saying "since Linux 2.6.39"
and "since Linux 2.6.38" for these flags seems unneeded.

> The
> .I mask
> argument to
> .BR statx ()
> is used to tell the kernel which fields the caller is interested in
> .I mask
> is an OR'ed combination of the following constants:

Missing a period between "interested in" and "mask".

> .B "Do not"
> simply set
> .I mask
> to UINT_MAX as one or more bits may, in future, be used to specify an extension
> to the buffer.

"in future" => "in the future".

> .I Note:
> For performance and simplicity reasons, different fields in the
> .I statx
> structure may contain state information from different moments
> during the execution of the system call.
> For example, if
> .IR stx_mode
> or
> .IR stx_uid
> is changed by another process by calling
> .BR chmod (2)
> or
> .BR chown (2),
> .BR stat ()
> might return the old
> .I stx_mode
> together with the new
> .IR stx_uid ,
> or the old
> .I stx_uid
> together with the new
> .IR stx_mode .

This example is confusing because chmod() doesn't change st_uid, and chown()
doesn't ordinarily change st_mode.  A better example would be that if chown() is
used to change both the owner and group, it's possible that statx() would return
the new owner paired with the old group, or the new group paired with the old
owner.  Or, the new owner and group could be observed before the new ctime.
Also, it seems this note belongs in a BUGS section of the man page, since it is
a bug.

> .SH ERRORS
> .TP
> .B EINVAL
> Invalid flag specified in
> .IR flags .

Actually, the behavior when specifying invalid flags differs depending on
whether a path is specified.

If a path is specified, then statx() fails with EINVAL if unrecognized flags are
specified.

However, the behavior differs when no path is specified.  For example, the the
following call with invalid flags succeeds:

	statx(fd, NULL, 0xFFFF0000, 0, &stbuf);

I think this is a bug, and it should be fixed by applying the same 'flags'
validation regardless of whether a path is specified.

> .B ENOENT
> A component of
> .I pathname
> does not exist, or
> .I pathname
> is an empty string.

is an empty string and AT_EMPTY_PATH was not specified in flags.

> .SH SEE ALSO
> .BR ls (1),
> .BR stat (1),

Shouldn't this link to stat (2), not stat (1)?

- Eric

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
                     ` (3 preceding siblings ...)
  2017-03-07 22:55   ` Eric Biggers
@ 2017-03-08  3:45   ` Eryu Guan
  2017-03-08  9:24   ` David Howells
                     ` (6 subsequent siblings)
  11 siblings, 0 replies; 38+ messages in thread
From: Eryu Guan @ 2017-03-08  3:45 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, mtk.manpages, Darrick J. Wong, linux-fsdevel, xfs

On Tue, Mar 07, 2017 at 05:22:55PM +0000, David Howells wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > This would be great to have in 4.11 together with the initial statx
> > implementation.  But until I see documentation and testcases for statx
> > I don't really feel comfortable reviewing anything related to it.
> 
> Well, since you asked for documentation, here's a manual page for you to
> review:-)
> 
> Note that as it isn't in glibc yet, I've left out all the
> set-this-and-that-#define-to-make-it-appear stuff except where it is pertinent
> to particular constants.
> 
> I don't suppose you know where the documentation on writing xfstests tests is?
> xfstests-dev/doc/ only contains an old and out of date changelog.

There's a "ADDING TO THE FSQA SUITE" section in README file, basically
it's telling you to use './new' script to generate new test template. So
based on what type of new test you're going to create, you could run:

./new generic	# new generic test
./new xfs	# new xfs test
./new ext4	# new ext4 test
./new shared	# new shared test
...

It finds the first free seq number and creates test template and a
default .out file in tests/<type> dir and add a new entry in
tests/<type>/group file.  You could just edit the template and update
the .out file with expected output from the test run.

E.g. currently 416 is the first free seq number in generic tests,
./new generic will create

./tests/generic/416
./tests/generic/416.out

I think statx test should be a new generic test.

Thanks,
Eryu

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
                     ` (4 preceding siblings ...)
  2017-03-08  3:45   ` Eryu Guan
@ 2017-03-08  9:24   ` David Howells
  2017-03-08 15:26     ` Jeff Layton
  2017-03-08  9:41   ` David Howells
                     ` (5 subsequent siblings)
  11 siblings, 1 reply; 38+ messages in thread
From: David Howells @ 2017-03-08  9:24 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: dhowells, Christoph Hellwig, Michael Kerrisk (man-pages),
	Darrick J. Wong, linux-fsdevel, xfs

Andreas Dilger <adilger@dilger.ca> wrote:

> > (Note that reserved space and padding is ommitted)
> 
> Do you think that not including the padding could be problematic for users?

Interesting question.  Do you think it would be?  I don't see why it should be
a problem, since they shouldn't be touching it anyway.  Also, the stat(2)
manpage also gives the same warning and omits padding and doesn't seem to have
been amended.

But if you would prefer, I can put it back.

The only reason I can really see for explicitly including it is to say that
this is where future extensions will go.

> > STATX_ATTR_IMMUTABLE	File is marked immutable
> ...
> > STATX_ATTR_APPEND	File is append-only

I should copy the definitions in chattr(1).

>    To test whether a path is a regular file (for example), one could write:
>    .nf
>    .in +4n
>    rc = statx(AT_FDCWD, pathname, 0, STATX_TYPE, &stx);
>    if (stx.stx_mask & STATX_TYPE && S_ISREG(sb.stx_mode)) {
>       /* Handle regular file */
>    }

Good idea.

> > Because tests of the above form are common, additional macros are defined by
> > POSIX to allow the test of the file type in
> > .I stx_mode
> > to be written more concisely:
> 
> Should this all just reference the existing stat(2) man page instead of
> duplicating the whole contents here?  This is spending a lot of space
> discussing the stx_mode field which could be avoided.

Possibly.  On the other hand, it means that everything you need to refer to is
in one page from the user's PoV.  I'm not sure what the best policy on this
is.

If I do defer to stat(2), I do need to make some sort of note that the
examples are different and the field name is stx_mask, not st_mask.

David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
                     ` (5 preceding siblings ...)
  2017-03-08  9:24   ` David Howells
@ 2017-03-08  9:41   ` David Howells
  2017-03-10  5:01     ` Eric Biggers
  2017-03-09  6:46   ` David Howells
                     ` (4 subsequent siblings)
  11 siblings, 1 reply; 38+ messages in thread
From: David Howells @ 2017-03-08  9:41 UTC (permalink / raw)
  To: Eric Biggers
  Cc: dhowells, Christoph Hellwig, mtk.manpages, Darrick J. Wong,
	linux-fsdevel, xfs

Eric Biggers <ebiggers3@gmail.com> wrote:

> > .I Note:
> > For performance and simplicity reasons, different fields in the
> > .I statx
> > structure may contain state information from different moments
> > during the execution of the system call.
> > For example, if
> > .IR stx_mode
> > or
> > .IR stx_uid
> > is changed by another process by calling
> > .BR chmod (2)
> > or
> > .BR chown (2),
> > .BR stat ()
> > might return the old
> > .I stx_mode
> > together with the new
> > .IR stx_uid ,
> > or the old
> > .I stx_uid
> > together with the new
> > .IR stx_mode .
> 
> This example is confusing because chmod() doesn't change st_uid, and chown()
> doesn't ordinarily change st_mode.

Doesn't chown() clear the S_ISUID and S_ISGID flags?

> A better example would be that if chown() is used to change both the owner
> and group, it's possible that statx() would return the new owner paired with
> the old group, or the new group paired with the old owner.  Or, the new
> owner and group could be observed before the new ctime.

This is copied more or less verbatim from stat(2), so what you said applies
there too.

> Also, it seems this note belongs in a BUGS section of the man page, since it
> is a bug.

No, it's not a bug, exactly.  It's a design decision.  It might be worth
adding an AT_STAT_LOCK flag to synchronise against attribute setting, but it
will cost you something in terms of performance - and even then, over a
network filesystem it might not achieve anything.

> > .SH ERRORS
> > .TP
> > .B EINVAL
> > Invalid flag specified in
> > .IR flags .
> 
> Actually, the behavior when specifying invalid flags differs depending on
> whether a path is specified.

Good point.  Should I reject AT_EMPTY_PATH, AT_NO_AUTOMOUNT and
AT_SYMLINK_NOFOLLOW if pathname is NULL, I wonder?

> > .B ENOENT
> > A component of
> > .I pathname
> > does not exist, or
> > .I pathname
> > is an empty string.
> 
> is an empty string and AT_EMPTY_PATH was not specified in flags.

That needs fixing in stat(2) also.

> > .SH SEE ALSO
> > .BR ls (1),
> > .BR stat (1),
> 
> Shouldn't this link to stat (2), not stat (1)?

Depends whether stat(1) ends up using statx() or not.  I can take it out for
now.

David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-08  9:24   ` David Howells
@ 2017-03-08 15:26     ` Jeff Layton
  2017-03-20 16:01       ` Matthew Wilcox
  0 siblings, 1 reply; 38+ messages in thread
From: Jeff Layton @ 2017-03-08 15:26 UTC (permalink / raw)
  To: David Howells, Andreas Dilger
  Cc: Christoph Hellwig, Michael Kerrisk (man-pages),
	Darrick J. Wong, linux-fsdevel, xfs

On Wed, 2017-03-08 at 09:24 +0000, David Howells wrote:
> Andreas Dilger <adilger@dilger.ca> wrote:
> 
> > > (Note that reserved space and padding is ommitted)
> > 
> > Do you think that not including the padding could be problematic for users?
> 
> Interesting question.  Do you think it would be?  I don't see why it should be
> a problem, since they shouldn't be touching it anyway.  Also, the stat(2)
> manpage also gives the same warning and omits padding and doesn't seem to have
> been amended.
> 
> But if you would prefer, I can put it back.
> 
> The only reason I can really see for explicitly including it is to say that
> this is where future extensions will go.
> 

It seems like we really ought to have 2 statx manpages:

A small statx(2) manpage that describes the kernel<->userland interface
(showing structs with padding), and then the "real" statx(3) manpage
that shows the glibc userland API.

The kernel<->userland one could be very small, and just say "This is not
the function you're interested in. Look at statx(3) for the C library
interface.", similar to what readdir(2) does.


> > > STATX_ATTR_IMMUTABLE	File is marked immutable
> > 
> > ...
> > > STATX_ATTR_APPEND	File is append-only
> 
> I should copy the definitions in chattr(1).
> 
> >    To test whether a path is a regular file (for example), one could write:
> >    .nf
> >    .in +4n
> >    rc = statx(AT_FDCWD, pathname, 0, STATX_TYPE, &stx);
> >    if (stx.stx_mask & STATX_TYPE && S_ISREG(sb.stx_mode)) {
> >       /* Handle regular file */
> >    }
> 
> Good idea.

Note that & and && have similar precedence, IIRC, so you probably want
to wrap that flag check in parenthesis.

> 
> > > Because tests of the above form are common, additional macros are defined by
> > > POSIX to allow the test of the file type in
> > > .I stx_mode
> > > to be written more concisely:
> > 
> > Should this all just reference the existing stat(2) man page instead of
> > duplicating the whole contents here?  This is spending a lot of space
> > discussing the stx_mode field which could be avoided.
> 
> Possibly.  On the other hand, it means that everything you need to refer to is
> in one page from the user's PoV.  I'm not sure what the best policy on this
> is.
> 
> If I do defer to stat(2), I do need to make some sort of note that the
> examples are different and the field name is stx_mask, not st_mask.
> 

If the plan is to eventually (in some far away future) to deprecate
stat(), then we probably don't want this manpage to refer to it as a
canonical source of information. I say duplicate it here.

It might also not hurt to consider updating the stat(2) manpage with a
NOTE that points to the statx manpage as well.

-- 
Jeff Layton <jlayton@poochiereds.net>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
                     ` (6 preceding siblings ...)
  2017-03-08  9:41   ` David Howells
@ 2017-03-09  6:46   ` David Howells
  2017-03-09  6:59     ` Eryu Guan
                       ` (2 more replies)
  2017-03-24 20:53   ` Eric Biggers
                     ` (3 subsequent siblings)
  11 siblings, 3 replies; 38+ messages in thread
From: David Howells @ 2017-03-09  6:46 UTC (permalink / raw)
  To: Eryu Guan
  Cc: dhowells, Christoph Hellwig, mtk.manpages, Darrick J. Wong,
	linux-fsdevel, xfs

Eryu Guan <eguan@redhat.com> wrote:

> There's a "ADDING TO THE FSQA SUITE" section in README file, basically
> it's telling you to use './new' script to generate new test template. So
> based on what type of new test you're going to create, you could run:
> 
> ./new generic	# new generic test
> ./new xfs	# new xfs test
> ./new ext4	# new ext4 test
> ./new shared	# new shared test
> ...
> 
> It finds the first free seq number and creates test template and a
> default .out file in tests/<type> dir and add a new entry in
> tests/<type>/group file.  You could just edit the template and update
> the .out file with expected output from the test run.

Yes, I'd spotted the ./new script, but it's not as simple as that since shell
scripts can't call system calls directly.

David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-09  6:46   ` David Howells
@ 2017-03-09  6:59     ` Eryu Guan
  2017-03-09  6:59     ` Darrick J. Wong
  2017-03-09  7:45     ` David Howells
  2 siblings, 0 replies; 38+ messages in thread
From: Eryu Guan @ 2017-03-09  6:59 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, mtk.manpages, Darrick J. Wong, linux-fsdevel, xfs

On Thu, Mar 09, 2017 at 06:46:48AM +0000, David Howells wrote:
> Eryu Guan <eguan@redhat.com> wrote:
> 
> > There's a "ADDING TO THE FSQA SUITE" section in README file, basically
> > it's telling you to use './new' script to generate new test template. So
> > based on what type of new test you're going to create, you could run:
> > 
> > ./new generic	# new generic test
> > ./new xfs	# new xfs test
> > ./new ext4	# new ext4 test
> > ./new shared	# new shared test
> > ...
> > 
> > It finds the first free seq number and creates test template and a
> > default .out file in tests/<type> dir and add a new entry in
> > tests/<type>/group file.  You could just edit the template and update
> > the .out file with expected output from the test run.
> 
> Yes, I'd spotted the ./new script, but it's not as simple as that since shell
> scripts can't call system calls directly.

A normal way we take for fstests is adding new syscall support to xfs_io
first, then writing new tests based on it, as some other new
syscalls/utilities do, e.g. set|get_encpolicy were added to xfs_io
in January to set|get encryption policy for ext4 encryption testing.

Thanks,
Eryu

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-09  6:46   ` David Howells
  2017-03-09  6:59     ` Eryu Guan
@ 2017-03-09  6:59     ` Darrick J. Wong
  2017-03-09 14:01       ` Christoph Hellwig
  2017-03-09  7:45     ` David Howells
  2 siblings, 1 reply; 38+ messages in thread
From: Darrick J. Wong @ 2017-03-09  6:59 UTC (permalink / raw)
  To: David Howells
  Cc: Eryu Guan, Christoph Hellwig, mtk.manpages, linux-fsdevel, xfs

On Thu, Mar 09, 2017 at 06:46:48AM +0000, David Howells wrote:
> Eryu Guan <eguan@redhat.com> wrote:
> 
> > There's a "ADDING TO THE FSQA SUITE" section in README file, basically
> > it's telling you to use './new' script to generate new test template. So
> > based on what type of new test you're going to create, you could run:
> > 
> > ./new generic	# new generic test
> > ./new xfs	# new xfs test
> > ./new ext4	# new ext4 test
> > ./new shared	# new shared test
> > ...
> > 
> > It finds the first free seq number and creates test template and a
> > default .out file in tests/<type> dir and add a new entry in
> > tests/<type>/group file.  You could just edit the template and update
> > the .out file with expected output from the test run.
> 
> Yes, I'd spotted the ./new script, but it's not as simple as that since shell
> scripts can't call system calls directly.

Probably best to place a program in src/ so that xfstests can make the
raw syscalls early while we wait for glibc to invent a wrapper.

--D

> 
> David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-09  6:46   ` David Howells
  2017-03-09  6:59     ` Eryu Guan
  2017-03-09  6:59     ` Darrick J. Wong
@ 2017-03-09  7:45     ` David Howells
  2 siblings, 0 replies; 38+ messages in thread
From: David Howells @ 2017-03-09  7:45 UTC (permalink / raw)
  To: Eryu Guan
  Cc: dhowells, Christoph Hellwig, mtk.manpages, Darrick J. Wong,
	linux-fsdevel, xfs

Eryu Guan <eguan@redhat.com> wrote:

> A normal way we take for fstests is adding new syscall support to xfs_io
> first, then writing new tests based on it, as some other new
> syscalls/utilities do, e.g. set|get_encpolicy were added to xfs_io
> in January to set|get encryption policy for ext4 encryption testing.

Please add this to the docs.

Thanks,
David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-09  6:59     ` Darrick J. Wong
@ 2017-03-09 14:01       ` Christoph Hellwig
  0 siblings, 0 replies; 38+ messages in thread
From: Christoph Hellwig @ 2017-03-09 14:01 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: David Howells, Eryu Guan, Christoph Hellwig, mtk.manpages,
	linux-fsdevel, xfs

On Wed, Mar 08, 2017 at 10:59:49PM -0800, Darrick J. Wong wrote:
> Probably best to place a program in src/ so that xfstests can make the
> raw syscalls early while we wait for glibc to invent a wrapper.

Іn general don't invent new programs if we can avoid it and add a statx
command to xfs_io, that all the tests can be built around.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-08  9:41   ` David Howells
@ 2017-03-10  5:01     ` Eric Biggers
  0 siblings, 0 replies; 38+ messages in thread
From: Eric Biggers @ 2017-03-10  5:01 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, mtk.manpages, Darrick J. Wong, linux-fsdevel, xfs

On Wed, Mar 08, 2017 at 09:41:00AM +0000, David Howells wrote:
> > 
> > This example is confusing because chmod() doesn't change st_uid, and chown()
> > doesn't ordinarily change st_mode.
> 
> Doesn't chown() clear the S_ISUID and S_ISGID flags?
> 

It can, but it's not a straightforward example.

> 
> No, it's not a bug, exactly.  It's a design decision.  It might be worth
> adding an AT_STAT_LOCK flag to synchronise against attribute setting, but it
> will cost you something in terms of performance - and even then, over a
> network filesystem it might not achieve anything.
> 

Well, regardless of whether people want to fix it or not, I do think it's a bug.
Currently even the update of a timestamp is non-atomic, since that involves
updating both tv_sec and tv_nsec.  Therefore, stat() might return the new tv_sec
along with the old tv_nsec, or vice versa.  This can cause some very strange
behavior, such as two successively observed timestamps going backwards in time.

I expect that historically people haven't complained enough for this to be
worthwhile to fix.  But in theory I think it could be fixed with little
performance loss by protecting all the stat data with a sequence count, similar
to how reads of i_size are made atomic on 32-bit platforms by using
i_size_seqcount.

> 
> Good point.  Should I reject AT_EMPTY_PATH, AT_NO_AUTOMOUNT and
> AT_SYMLINK_NOFOLLOW if pathname is NULL, I wonder?
> 

It could be argued either way, but I was thinking those particular flags should
just be ignored, as they have known meanings but simply aren't relevant.  It
could also cause confusion for

	statx(dfd, "", AT_EMPTY_PATH, 0, buffer)

to succeed but for

	statx(dfd, NULL, AT_EMPTY_PATH, 0, buffer)

to fail (for example).

- Eric

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-08 15:26     ` Jeff Layton
@ 2017-03-20 16:01       ` Matthew Wilcox
  2017-03-22 10:55         ` Jeff Layton
  0 siblings, 1 reply; 38+ messages in thread
From: Matthew Wilcox @ 2017-03-20 16:01 UTC (permalink / raw)
  To: Jeff Layton
  Cc: David Howells, Andreas Dilger, Christoph Hellwig,
	Michael Kerrisk (man-pages),
	Darrick J. Wong, linux-fsdevel, xfs

On Wed, Mar 08, 2017 at 10:26:14AM -0500, Jeff Layton wrote:
> On Wed, 2017-03-08 at 09:24 +0000, David Howells wrote:
> > Andreas Dilger <adilger@dilger.ca> wrote:
> > >    To test whether a path is a regular file (for example), one could write:
> > >    .nf
> > >    .in +4n
> > >    rc = statx(AT_FDCWD, pathname, 0, STATX_TYPE, &stx);
> > >    if (stx.stx_mask & STATX_TYPE && S_ISREG(sb.stx_mode)) {
> > >       /* Handle regular file */
> > >    }
> > 
> > Good idea.
> 
> Note that & and && have similar precedence, IIRC, so you probably want
> to wrap that flag check in parenthesis.

They're actually pretty far apart.  & is priority 8 and && is priority 11.
But the fact that you thought they were similar shows that most of
us don't really know what C's operator precedence is (beyond + and *)
and we should err on the side of clarity by using the unnecessary parens.

> If the plan is to eventually (in some far away future) to deprecate
> stat(), then we probably don't want this manpage to refer to it as a
> canonical source of information. I say duplicate it here.

We haven't even deprecated select() in favour of poll() ... indeed,
pselect() was added.  I can't imagine stat() ever being deprecated.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-20 16:01       ` Matthew Wilcox
@ 2017-03-22 10:55         ` Jeff Layton
  0 siblings, 0 replies; 38+ messages in thread
From: Jeff Layton @ 2017-03-22 10:55 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: David Howells, Andreas Dilger, Christoph Hellwig,
	Michael Kerrisk (man-pages),
	Darrick J. Wong, linux-fsdevel, xfs

On Mon, 2017-03-20 at 09:01 -0700, Matthew Wilcox wrote:
> On Wed, Mar 08, 2017 at 10:26:14AM -0500, Jeff Layton wrote:
> > On Wed, 2017-03-08 at 09:24 +0000, David Howells wrote:
> > > Andreas Dilger <adilger@dilger.ca> wrote:
> > > >    To test whether a path is a regular file (for example), one could write:
> > > >    .nf
> > > >    .in +4n
> > > >    rc = statx(AT_FDCWD, pathname, 0, STATX_TYPE, &stx);
> > > >    if (stx.stx_mask & STATX_TYPE && S_ISREG(sb.stx_mode)) {
> > > >       /* Handle regular file */
> > > >    }
> > > 
> > > Good idea.
> > 
> > Note that & and && have similar precedence, IIRC, so you probably want
> > to wrap that flag check in parenthesis.
> 
> They're actually pretty far apart.  & is priority 8 and && is priority 11.
> But the fact that you thought they were similar shows that most of
> us don't really know what C's operator precedence is (beyond + and *)
> and we should err on the side of clarity by using the unnecessary parens.
> 

Ahh, good to know. My rules are generally: multiplication and division
come before addition and subtraction, and put parenthesis around
everything else. :)

> > If the plan is to eventually (in some far away future) to deprecate
> > stat(), then we probably don't want this manpage to refer to it as a
> > canonical source of information. I say duplicate it here.
> 
> We haven't even deprecated select() in favour of poll() ... indeed,
> pselect() was added.  I can't imagine stat() ever being deprecated.

Ok, deprecated is probably the wrong choice of words. You're correct
that we'll never be able to fully rip it out just due to the volume of
old programs that use it.

My thinking is that statx will come to supplant stat in newer code, and
stat will eventually become a "legacy" syscall. Typically we don't want
to have to refer to "legacy" manpages for canonical info in new
interfaces. If anything we may want to remove that info from stat(2)
manpage and have it refer to statx(2) for it.
-- 
Jeff Layton <jlayton@poochiereds.net>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
                     ` (7 preceding siblings ...)
  2017-03-09  6:46   ` David Howells
@ 2017-03-24 20:53   ` Eric Biggers
  2017-03-27  0:46     ` Andreas Dilger
  2017-03-27  9:55   ` statx manpage David Howells
                     ` (2 subsequent siblings)
  11 siblings, 1 reply; 38+ messages in thread
From: Eric Biggers @ 2017-03-24 20:53 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, mtk.manpages, Darrick J. Wong, linux-fsdevel, xfs

On Tue, Mar 07, 2017 at 05:22:55PM +0000, David Howells wrote:
> STATX_ALL	[All currently available stuff]
> .TE
> .in
> .PP
> .B "Do not"
> simply set
> .I mask
> to UINT_MAX as one or more bits may, in future, be used to specify an extension
> to the buffer.

To clarify, will an "extension to the buffer" be an increase in the size of
struct statx?  I think it would have to be, otherwise programs filling a struct
statx with STATX_ALL would start breaking as soon as they're rebuilt with the
new value of STATX_ALL, no?  Or would these "extension to the buffer" bits not
be added to STATX_ALL ...?

And I don't suppose there's anything we can do to stop programs from asking for
mask bits that haven't been defined yet, then breaking later if they happen to
be defined as "extensions"?  Maybe adding an extra "buffer size" argument to the
syscall?

I'm concerned that the idea of "extensions" isn't well thought out, and in
practice we'll just be stuck with the current struct size (256 bytes) forever.

- Eric

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-24 20:53   ` Eric Biggers
@ 2017-03-27  0:46     ` Andreas Dilger
  2017-03-27 15:40       ` Darrick J. Wong
  2017-03-27 16:25       ` David Howells
  0 siblings, 2 replies; 38+ messages in thread
From: Andreas Dilger @ 2017-03-27  0:46 UTC (permalink / raw)
  To: Eric Biggers
  Cc: David Howells, Christoph Hellwig, Michael Kerrisk (man-pages),
	Darrick J. Wong, linux-fsdevel, xfs

[-- Attachment #1: Type: text/plain, Size: 2391 bytes --]


> On Mar 24, 2017, at 2:53 PM, Eric Biggers <ebiggers3@gmail.com> wrote:
> 
> On Tue, Mar 07, 2017 at 05:22:55PM +0000, David Howells wrote:
>> STATX_ALL	[All currently available stuff]
>> .TE
>> .in
>> .PP
>> .B "Do not"
>> simply set
>> .I mask
>> to UINT_MAX as one or more bits may, in future, be used to specify an extension
>> to the buffer.
> 
> To clarify, will an "extension to the buffer" be an increase in the size of
> struct statx?  I think it would have to be, otherwise programs filling a struct
> statx with STATX_ALL would start breaking as soon as they're rebuilt with the
> new value of STATX_ALL, no?  Or would these "extension to the buffer" bits not
> be added to STATX_ALL ...?

The value of STATX_ALL would match the size of struct statx in the header at
compilation time, so this would always be consistent.
> 
> And I don't suppose there's anything we can do to stop programs from asking
> for mask bits that haven't been defined yet, then breaking later if they
> happen to be defined as "extensions"?  Maybe adding an extra "buffer size"
> argument to the syscall?

You can't stop applications from doing dumb things, like asking to read 1MB
of data into a buffer that is only 512KB in size.  That will also work fine
as long as the application only reads a files smaller than 512KB.

Similarly, if the statx() API says that the STATX_ALL mask is the list of
currently-supported bits, but the app asks for more bits than it allocates
a buffer for, there isn't much that the kernel can do.

> I'm concerned that the idea of "extensions" isn't well thought out, and in
> practice we'll just be stuck with the current struct size (256 bytes) forever.

The extensions work exactly as they should - the client sets bits for fields
that it needs (and by definition it shouldn't ask for anything that it doesn't
understand), and the kernel masks this down to the bits that it understands.

If the client asks for more bits than the kernel understands, it is likely a
newer application on an older kernel, and it will only get back the fields
that the kernel understands.  The reverse (client asking for fewer bits than
the kernel understands) is normal behaviour for this interface.  The kernel should only fill in fields that the client requested and for which there is
space in the struct.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
                     ` (8 preceding siblings ...)
  2017-03-24 20:53   ` Eric Biggers
@ 2017-03-27  9:55   ` David Howells
  2017-03-31 15:56   ` Christoph Hellwig
  2017-03-31 16:43   ` David Howells
  11 siblings, 0 replies; 38+ messages in thread
From: David Howells @ 2017-03-27  9:55 UTC (permalink / raw)
  To: Eric Biggers
  Cc: dhowells, Christoph Hellwig, mtk.manpages, Darrick J. Wong,
	linux-fsdevel, xfs

Eric Biggers <ebiggers3@gmail.com> wrote:

> On Tue, Mar 07, 2017 at 05:22:55PM +0000, David Howells wrote:
> > STATX_ALL	[All currently available stuff]
> > .TE
> > .in
> > .PP
> > .B "Do not"
> > simply set
> > .I mask
> > to UINT_MAX as one or more bits may, in future, be used to specify an extension
> > to the buffer.
> 
> To clarify, will an "extension to the buffer" be an increase in the size of
> struct statx?

Yes.  I can make that more explicit, say perhaps "... specify an extension to
the size of the buffer".

> I think it would have to be, otherwise programs filling a struct statx with
> STATX_ALL would start breaking as soon as they're rebuilt with the new value
> of STATX_ALL, no?  Or would these "extension to the buffer" bits not be
> added to STATX_ALL ...?

It would have to work that way.  I can put this in a comment for future
guidance in the kernel header file if you'd prefer.

> And I don't suppose there's anything we can do to stop programs from asking
> for mask bits that haven't been defined yet, then breaking later if they
> happen to be defined as "extensions"?

If someone decides not to read the documentation, one could argue they get
what they deserve.

> Maybe adding an extra "buffer size" argument to the syscall?

This idea has already been rejected.

> I'm concerned that the idea of "extensions" isn't well thought out, and in
> practice we'll just be stuck with the current struct size (256 bytes) forever.

One or more bits (probably just one) are tentatively reserved to indicate that
the buffer is now bigger.  How much bigger I cannot say at this point, not
knowing the circumstances yet that will require the extension.

David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-27  0:46     ` Andreas Dilger
@ 2017-03-27 15:40       ` Darrick J. Wong
  2017-03-27 16:25       ` David Howells
  1 sibling, 0 replies; 38+ messages in thread
From: Darrick J. Wong @ 2017-03-27 15:40 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: Eric Biggers, David Howells, Christoph Hellwig,
	Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

On Sun, Mar 26, 2017 at 06:46:43PM -0600, Andreas Dilger wrote:
> 
> > On Mar 24, 2017, at 2:53 PM, Eric Biggers <ebiggers3@gmail.com> wrote:
> > 
> > On Tue, Mar 07, 2017 at 05:22:55PM +0000, David Howells wrote:
> >> STATX_ALL	[All currently available stuff]
> >> .TE
> >> .in
> >> .PP
> >> .B "Do not"
> >> simply set
> >> .I mask
> >> to UINT_MAX as one or more bits may, in future, be used to specify an extension
> >> to the buffer.
> > 
> > To clarify, will an "extension to the buffer" be an increase in the size of
> > struct statx?  I think it would have to be, otherwise programs filling a struct
> > statx with STATX_ALL would start breaking as soon as they're rebuilt with the
> > new value of STATX_ALL, no?  Or would these "extension to the buffer" bits not
> > be added to STATX_ALL ...?
> 
> The value of STATX_ALL would match the size of struct statx in the header at
> compilation time, so this would always be consistent.
> > 
> > And I don't suppose there's anything we can do to stop programs from asking
> > for mask bits that haven't been defined yet, then breaking later if they
> > happen to be defined as "extensions"?  Maybe adding an extra "buffer size"
> > argument to the syscall?
> 
> You can't stop applications from doing dumb things, like asking to read 1MB
> of data into a buffer that is only 512KB in size.  That will also work fine
> as long as the application only reads a files smaller than 512KB.
> 
> Similarly, if the statx() API says that the STATX_ALL mask is the list of
> currently-supported bits, but the app asks for more bits than it allocates
> a buffer for, there isn't much that the kernel can do.
> 
> > I'm concerned that the idea of "extensions" isn't well thought out, and in
> > practice we'll just be stuck with the current struct size (256 bytes) forever.
> 
> The extensions work exactly as they should - the client sets bits for fields
> that it needs (and by definition it shouldn't ask for anything that it doesn't
> understand), and the kernel masks this down to the bits that it understands.
> 
> If the client asks for more bits than the kernel understands, it is likely a
> newer application on an older kernel, and it will only get back the fields
> that the kernel understands.  The reverse (client asking for fewer bits than
> the kernel understands) is normal behaviour for this interface.  The kernel
> should only fill in fields that the client requested and for which there is
> space in the struct.

During the statx session at LSF last week I asked if filesystems ought
to fill in fields that weren't asked for (btime, specifically) and the
impression I got was that it's ok to go ahead and fill out fields that
weren't asked for if we already have the data.

Since statx backends can do that, they'll have to check the structure
size, and not rely on "you asked for this field so we assume that you
allocated enough memory in userspace to hold it".

Or we could just shift the precedent now -- programs only get the
information they ask for, and in asking for it we assume that we can
write to that part of the buffer.  Frankly I'd prefer that behavior (see
the XFS statx patch), but I don't own the interface. :)

--D

> 
> Cheers, Andreas
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-27  0:46     ` Andreas Dilger
  2017-03-27 15:40       ` Darrick J. Wong
@ 2017-03-27 16:25       ` David Howells
  2017-03-27 16:46         ` Christoph Hellwig
                           ` (2 more replies)
  1 sibling, 3 replies; 38+ messages in thread
From: David Howells @ 2017-03-27 16:25 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: dhowells, Andreas Dilger, Eric Biggers, Christoph Hellwig,
	Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

Darrick J. Wong <darrick.wong@oracle.com> wrote:

> During the statx session at LSF last week I asked if filesystems ought
> to fill in fields that weren't asked for (btime, specifically) and the
> impression I got was that it's ok to go ahead and fill out fields that
> weren't asked for if we already have the data.

Yes.  The filesystem may return data that it wasn't asked for.

> Since statx backends can do that, they'll have to check the structure
> size, and not rely on "you asked for this field so we assume that you
> allocated enough memory in userspace to hold it".

Kind of.  I'm assuming that the extension fields will be in struct kstat and
can be set directly by the filesystem unconditionally, and the core can choose
whether to copy them or not.  However, assuming that one of the mask bits is
reserved to indicate a buffer extension, this will be made available to the fs
to let the fs know whether it's pointless to return it.

However, at the moment I have no extensions to play with.

Would it help you if I marked one of the mask bits reserved for this purpose?

> Or we could just shift the precedent now -- programs only get the
> information they ask for, and in asking for it we assume that we can
> write to that part of the buffer.  Frankly I'd prefer that behavior (see
> the XFS statx patch), but I don't own the interface. :)

That means a filesystem can't simply return non-basic data unconditionally if
possible.  I prefer letting it do so if it doesn't incur any extra I/O
overheads.

David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-27 16:25       ` David Howells
@ 2017-03-27 16:46         ` Christoph Hellwig
  2017-03-27 19:04           ` Jeff Layton
  2017-03-27 18:57         ` David Howells
  2017-03-28  0:13         ` [PATCH] Add initial batch of statx() LTP tests David Howells
  2 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2017-03-27 16:46 UTC (permalink / raw)
  To: David Howells
  Cc: Darrick J. Wong, Andreas Dilger, Eric Biggers, Christoph Hellwig,
	Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

On Mon, Mar 27, 2017 at 05:25:26PM +0100, David Howells wrote:
> That means a filesystem can't simply return non-basic data unconditionally if
> possible.  I prefer letting it do so if it doesn't incur any extra I/O
> overheads.

This seems like it will lead to userspace expecting certain fields to
just be there, and a lot harder to properly verify for tests.  Which btw
we all need for these odd behaviors.  If we can't get them any time soon
(e.g. before -rc6) I think we'll simply have to revert statx instead of
leaving this untested mess in the tree.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-27 16:25       ` David Howells
  2017-03-27 16:46         ` Christoph Hellwig
@ 2017-03-27 18:57         ` David Howells
  2017-03-28  0:13         ` [PATCH] Add initial batch of statx() LTP tests David Howells
  2 siblings, 0 replies; 38+ messages in thread
From: David Howells @ 2017-03-27 18:57 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: dhowells, Darrick J. Wong, Andreas Dilger, Eric Biggers,
	Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

Christoph Hellwig <hch@infradead.org> wrote:

> On Mon, Mar 27, 2017 at 05:25:26PM +0100, David Howells wrote:
> > That means a filesystem can't simply return non-basic data unconditionally if
> > possible.  I prefer letting it do so if it doesn't incur any extra I/O
> > overheads.
> 
> This seems like it will lead to userspace expecting certain fields to
> just be there, and a lot harder to properly verify for tests.  Which btw
> we all need for these odd behaviors.  If we can't get them any time soon
> (e.g. before -rc6) I think we'll simply have to revert statx instead of
> leaving this untested mess in the tree.

Have you reviewed the manpage yet?

David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-27 16:46         ` Christoph Hellwig
@ 2017-03-27 19:04           ` Jeff Layton
  2017-03-28 19:39             ` Andreas Dilger
  2017-03-31 15:49             ` Christoph Hellwig
  0 siblings, 2 replies; 38+ messages in thread
From: Jeff Layton @ 2017-03-27 19:04 UTC (permalink / raw)
  To: Christoph Hellwig, David Howells
  Cc: Darrick J. Wong, Andreas Dilger, Eric Biggers,
	Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

On Mon, 2017-03-27 at 09:46 -0700, Christoph Hellwig wrote:
> On Mon, Mar 27, 2017 at 05:25:26PM +0100, David Howells wrote:
> > That means a filesystem can't simply return non-basic data unconditionally if
> > possible.  I prefer letting it do so if it doesn't incur any extra I/O
> > overheads.
> 
> This seems like it will lead to userspace expecting certain fields to
> just be there, and a lot harder to properly verify for tests.  Which btw
> we all need for these odd behaviors.  If we can't get them any time soon
> (e.g. before -rc6) I think we'll simply have to revert statx instead of
> leaving this untested mess in the tree.

I don't think so.

I think we just have to clearly document that that will not be the
case. If they really expect the field to be there, then they need to
set the bit in the "want" mask -- it's really as simple as that.

-- 
Jeff Layton <jlayton@poochiereds.net>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH] Add initial batch of statx() LTP tests
  2017-03-27 16:25       ` David Howells
  2017-03-27 16:46         ` Christoph Hellwig
  2017-03-27 18:57         ` David Howells
@ 2017-03-28  0:13         ` David Howells
  2017-03-28  6:28           ` Christoph Hellwig
  2017-03-28  8:23           ` David Howells
  2 siblings, 2 replies; 38+ messages in thread
From: David Howells @ 2017-03-28  0:13 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: dhowells, Darrick J. Wong, Andreas Dilger, Eric Biggers,
	Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

Christoph Hellwig <hch@infradead.org> wrote:

> > That means a filesystem can't simply return non-basic data unconditionally if
> > possible.  I prefer letting it do so if it doesn't incur any extra I/O
> > overheads.
> 
> This seems like it will lead to userspace expecting certain fields to
> just be there, and a lot harder to properly verify for tests.  Which btw
> we all need for these odd behaviors.  If we can't get them any time soon
> (e.g. before -rc6) I think we'll simply have to revert statx instead of
> leaving this untested mess in the tree.

Here you go.  First batch of tests.  Please review - and test:-)

David
---
commit fff886ec7ee53aa266a12d9dbb2825952fa1adb9
Author: David Howells <dhowells@redhat.com>
Date:   Mon Mar 27 21:42:32 2017 +0100

    Add initial batch of statx() LTP tests
    
    Add an initial batch of statx() tests for LTP, deriving them from all of
    the stat(), fstat() and lstat() tests.
    
    Note that since statx() is not yet implemented in glibc, a header file is
    added to provide the missing system call bits.
    
    Signed-off-by: David Howells <dhowells@redhat.com>

diff --git a/include/linux-statx.h b/include/linux-statx.h
new file mode 100644
index 0000000..d25fcbf
--- /dev/null
+++ b/include/linux-statx.h
@@ -0,0 +1,158 @@
+#ifndef _EXTENDED_LINUX_STAT_H
+#define _EXTENDED_LINUX_STAT_H
+
+#include <sys/stat.h>
+#include <linux/types.h>
+#include <linux/stat.h>
+
+#ifndef STATX_TYPE
+
+#ifndef __NR_statx
+#if defined(__x86_64__)
+#define __NR_statx 332
+#elif defined(__i386__)
+#define __NR_statx 383
+#endif
+#endif /* !__NR_statx */
+
+/*
+ * Timestamp structure for the timestamps in struct statx.
+ *
+ * tv_sec holds the number of seconds before (negative) or after (positive)
+ * 00:00:00 1st January 1970 UTC.
+ *
+ * tv_nsec holds a number of nanoseconds before (0..-999,999,999 if tv_sec is
+ * negative) or after (0..999,999,999 if tv_sec is positive) the tv_sec time.
+ *
+ * Note that if both tv_sec and tv_nsec are non-zero, then the two values must
+ * either be both positive or both negative.
+ *
+ * __reserved is held in case we need a yet finer resolution.
+ */
+struct statx_timestamp {
+	__s64	tv_sec;
+	__s32	tv_nsec;
+	__s32	__reserved;
+};
+
+/*
+ * Structures for the extended file attribute retrieval system call
+ * (statx()).
+ *
+ * The caller passes a mask of what they're specifically interested in as a
+ * parameter to statx().  What statx() actually got will be indicated in
+ * st_mask upon return.
+ *
+ * For each bit in the mask argument:
+ *
+ * - if the datum is not supported:
+ *
+ *   - the bit will be cleared, and
+ *
+ *   - the datum will be set to an appropriate fabricated value if one is
+ *     available (eg. CIFS can take a default uid and gid), otherwise
+ *
+ *   - the field will be cleared;
+ *
+ * - otherwise, if explicitly requested:
+ *
+ *   - the datum will be synchronised to the server if AT_STATX_FORCE_SYNC is
+ *     set or if the datum is considered out of date, and
+ *
+ *   - the field will be filled in and the bit will be set;
+ *
+ * - otherwise, if not requested, but available in approximate form without any
+ *   effort, it will be filled in anyway, and the bit will be set upon return
+ *   (it might not be up to date, however, and no attempt will be made to
+ *   synchronise the internal state first);
+ *
+ * - otherwise the field and the bit will be cleared before returning.
+ *
+ * Items in STATX_BASIC_STATS may be marked unavailable on return, but they
+ * will have values installed for compatibility purposes so that stat() and
+ * co. can be emulated in userspace.
+ */
+struct statx {
+	/* 0x00 */
+	__u32	stx_mask;	/* What results were written [uncond] */
+	__u32	stx_blksize;	/* Preferred general I/O size [uncond] */
+	__u64	stx_attributes;	/* Flags conveying information about the file [uncond] */
+	/* 0x10 */
+	__u32	stx_nlink;	/* Number of hard links */
+	__u32	stx_uid;	/* User ID of owner */
+	__u32	stx_gid;	/* Group ID of owner */
+	__u16	stx_mode;	/* File mode */
+	__u16	__spare0[1];
+	/* 0x20 */
+	__u64	stx_ino;	/* Inode number */
+	__u64	stx_size;	/* File size */
+	__u64	stx_blocks;	/* Number of 512-byte blocks allocated */
+	__u64	__spare1[1];
+	/* 0x40 */
+	struct statx_timestamp	stx_atime;	/* Last access time */
+	struct statx_timestamp	stx_btime;	/* File creation time */
+	struct statx_timestamp	stx_ctime;	/* Last attribute change time */
+	struct statx_timestamp	stx_mtime;	/* Last data modification time */
+	/* 0x80 */
+	__u32	stx_rdev_major;	/* Device ID of special file [if bdev/cdev] */
+	__u32	stx_rdev_minor;
+	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
+	__u32	stx_dev_minor;
+	/* 0x90 */
+	__u64	__spare2[14];	/* Spare space for future expansion */
+	/* 0x100 */
+};
+
+/*
+ * Flags to be stx_mask
+ *
+ * Query request/result mask for statx() and struct statx::stx_mask.
+ *
+ * These bits should be set in the mask argument of statx() to request
+ * particular items when calling statx().
+ */
+#define STATX_TYPE		0x00000001U	/* Want/got stx_mode & S_IFMT */
+#define STATX_MODE		0x00000002U	/* Want/got stx_mode & ~S_IFMT */
+#define STATX_NLINK		0x00000004U	/* Want/got stx_nlink */
+#define STATX_UID		0x00000008U	/* Want/got stx_uid */
+#define STATX_GID		0x00000010U	/* Want/got stx_gid */
+#define STATX_ATIME		0x00000020U	/* Want/got stx_atime */
+#define STATX_MTIME		0x00000040U	/* Want/got stx_mtime */
+#define STATX_CTIME		0x00000080U	/* Want/got stx_ctime */
+#define STATX_INO		0x00000100U	/* Want/got stx_ino */
+#define STATX_SIZE		0x00000200U	/* Want/got stx_size */
+#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 */
+
+/*
+ * Attributes to be found in stx_attributes
+ *
+ * These give information about the features or the state of a file that might
+ * be of use to ordinary userspace programs such as GUIs or ls rather than
+ * specialised tools.
+ *
+ * Note that the flags marked [I] correspond to generic FS_IOC_FLAGS
+ * semantically.  Where possible, the numerical value is picked to correspond
+ * also.
+ */
+#define STATX_ATTR_COMPRESSED		0x00000004 /* [I] File is compressed by the fs */
+#define STATX_ATTR_IMMUTABLE		0x00000010 /* [I] File is marked immutable */
+#define STATX_ATTR_APPEND		0x00000020 /* [I] File is append-only */
+#define STATX_ATTR_NODUMP		0x00000040 /* [I] File is not to be dumped */
+#define STATX_ATTR_ENCRYPTED		0x00000800 /* [I] File requires key to decrypt in fs */
+
+#define STATX_ATTR_AUTOMOUNT		0x00001000 /* Dir: Automount trigger */
+
+
+static inline
+ssize_t statx(int dfd, const char *filename, unsigned flags,
+	      unsigned int mask, struct statx *buffer)
+{
+	return syscall(__NR_statx, dfd, filename, flags, mask, buffer);
+}
+
+#endif /* STATX_TYPE */
+
+#endif /* _EXTENDED_LINUX_STAT_H */
diff --git a/include/tst_fs.h b/include/tst_fs.h
index 52befbe..c1885c4 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -201,4 +201,11 @@ static inline int tst_dir_is_empty(void (*cleanup)(void), const char *name, int
 }
 #endif
 
+/*
+ * Compare the contents of a statx struct with that of a stat struct and check
+ * that they're the same.
+ */
+struct statx;
+int tst_cmp_statx(const struct statx *stx, const struct stat *st);
+
 #endif	/* TST_FS_H__ */
diff --git a/lib/tst_cmp_stat.c b/lib/tst_cmp_stat.c
new file mode 100644
index 0000000..fe1427b
--- /dev/null
+++ b/lib/tst_cmp_stat.c
@@ -0,0 +1,73 @@
+/* Compare statx struct and stat struct contents.
+ *
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <sys/stat.h>
+#include "test.h"
+#include "linux-statx.h"
+
+/*
+ * Compare the contents of a statx struct with that of a stat struct and check
+ * that they're the same.
+ */
+int tst_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:
+	tst_resm(TINFO, "Mismatch between stx_%s and st_%s",
+		 what, what);
+	return -1;
+}
diff --git a/testcases/kernel/syscalls/statx/Makefile b/testcases/kernel/syscalls/statx/Makefile
new file mode 100644
index 0000000..bf12010
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/Makefile
@@ -0,0 +1,26 @@
+#
+#  Copyright (c) International Business Machines  Corp., 2001
+#
+#  This program is free software;  you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY;  without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+#  the GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program;  if not, write to the Free Software
+#  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(abs_srcdir)/../utils/newer_64.mk
+
+%_64: CPPFLAGS += -D_FILE_OFFSET_BITS=64
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/statx/statx01.c b/testcases/kernel/syscalls/statx/statx01.c
new file mode 100644
index 0000000..a33d23d
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx01.c
@@ -0,0 +1,166 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from stat01.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (c) International Business Machines  Corp., 2001
+ *  07/2001 John George
+ *
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program;  if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+/*
+ *  Verify that, statx(2) succeeds to get the status of a file and fills the
+ *  statx structure elements.
+ */
+#include <stdio.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <pwd.h>
+
+#include "test.h"
+#include "safe_macros.h"
+#include "linux-statx.h"
+
+#define FILE_MODE	0644
+#define TESTFILE	"testfile"
+#define FILE_SIZE       1024
+#define BUF_SIZE	256
+#define MASK		0777
+
+char *TCID = "statx01";
+int TST_TOTAL = 1;
+
+static uid_t user_id;
+static gid_t group_id;
+
+static void setup(void);
+static void cleanup(void);
+
+static void verify(void)
+{
+	struct statx statx_buf;
+	struct stat stat_buf;
+	int fail = 0;
+
+	TEST(statx(AT_FDCWD, TESTFILE, 0, STATX_BASIC_STATS, &statx_buf));
+
+	if (TEST_RETURN == -1) {
+		tst_resm(TFAIL | TTERRNO, "statx(%s) failed", TESTFILE);
+		return;
+	}
+
+	TEST(stat(TESTFILE, &stat_buf));
+	if (TEST_RETURN == -1) {
+		tst_resm(TFAIL | TTERRNO, "stat(%s) failed", TESTFILE);
+		return;
+	}
+
+	if (tst_cmp_statx(&statx_buf, &stat_buf) == -1)
+		return;
+
+	if (statx_buf.stx_uid != user_id) {
+		tst_resm(TINFO, "statx_buf.stx_uid = %i expected %i",
+		         statx_buf.stx_uid, user_id);
+		fail++;
+	}
+
+	if (statx_buf.stx_gid != group_id) {
+		tst_resm(TINFO, "statx_buf.stx_gid = %i expected %i",
+		         statx_buf.stx_gid, group_id);
+		fail++;
+	}
+
+	if (statx_buf.stx_size != FILE_SIZE) {
+		tst_resm(TINFO, "statx_buf.stx_size = %llu expected %i",
+		         statx_buf.stx_size, FILE_SIZE);
+		fail++;
+	}
+
+        if ((statx_buf.stx_mode & MASK) != FILE_MODE) {
+		tst_resm(TINFO, "statx_buf.stx_mode = %o expected %o",
+		         (statx_buf.stx_mode & MASK), FILE_MODE);
+		fail++;
+	}
+
+	if (fail) {
+		tst_resm(TFAIL, "functionality of statx incorrect");
+		return;
+	}
+
+	tst_resm(TPASS, "functionality of stat correct");
+}
+
+int main(int ac, char **av)
+{
+	int lc;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); lc++)
+		verify();
+
+	cleanup();
+	tst_exit();
+}
+
+static void setup(void)
+{
+	struct passwd *ltpuser;
+	char tst_buff[BUF_SIZE];
+	int wbytes;
+	int write_len = 0;
+	int fd;
+
+	tst_require_root();
+
+	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+
+	ltpuser = SAFE_GETPWNAM(NULL, "nobody");
+	SAFE_SETUID(NULL, ltpuser->pw_uid);
+
+	TEST_PAUSE;
+
+	tst_tmpdir();
+
+	umask(022);
+
+	fd = SAFE_OPEN(tst_rmdir, TESTFILE, O_WRONLY | O_CREAT, FILE_MODE);
+
+	/* Fill the test buffer with the known data */
+	memset(tst_buff, 'a', BUF_SIZE - 1);
+
+	/* Write to the file 1k data from the buffer */
+	while (write_len < FILE_SIZE) {
+		if ((wbytes = write(fd, tst_buff, sizeof(tst_buff))) <= 0)
+			tst_brkm(TBROK | TERRNO, cleanup, "write failed");
+		else
+			write_len += wbytes;
+	}
+
+	SAFE_CLOSE(tst_rmdir, fd);
+
+	user_id = getuid();
+	group_id = getgid();
+}
+
+static void cleanup(void)
+{
+	tst_rmdir();
+}
diff --git a/testcases/kernel/syscalls/statx/statx02.c b/testcases/kernel/syscalls/statx/statx02.c
new file mode 100644
index 0000000..9bcbea0
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx02.c
@@ -0,0 +1,243 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from stat02.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (c) International Business Machines  Corp., 2001
+ *
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program;  if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/*
+ * Test Name: statx02
+ *
+ * Test Description:
+ *  Verify that, statx(2) succeeds to get the status of a file and fills the
+ *  statx structure elements though process doesn't have read access to the
+ *  file.
+ *
+ * Expected Result:
+ *  statx() should return value 0 on success and the statx structure elements
+ *  should be filled with specified 'file' information.
+ *
+ * Algorithm:
+ *  Setup:
+ *   Setup signal handling.
+ *   Create temporary directory.
+ *   Pause for SIGUSR1 if option specified.
+ *
+ *  Test:
+ *   Loop if the proper options are given.
+ *   Execute system call
+ *   Check return code, if system call failed (return=-1)
+ *   	Log the errno and Issue a FAIL message.
+ *   Otherwise,
+ *   	Verify the Functionality of system call
+ *      if successful,
+ *      	Issue Functionality-Pass message.
+ *      Otherwise,
+ *		Issue Functionality-Fail message.
+ *  Cleanup:
+ *   Print errno log and/or timing stats if options given
+ *   Delete the temporary directory created.
+ *
+ * Usage:  <for command-line>
+ *  statx02 [-c n] [-e] [-f] [-i n] [-I x] [-p x] [-t]
+ *	where,  -c n : Run n copies concurrently.
+ *		-e   : Turn on errno logging.
+ *		-f   : Turn off functionality Testing.
+ *		-i n : Execute test n times.
+ *		-I x : Execute test for x seconds.
+ *		-P x : Pause for x seconds between iterations.
+ *		-t   : Turn on syscall timing.
+ *
+ * Restrictions:
+ *
+ */
+#include <stdio.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <pwd.h>
+
+#include "test.h"
+#include "linux-statx.h"
+
+#define FILE_MODE	S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
+#define TESTFILE	"testfile"
+#define FILE_SIZE       1024
+#define BUF_SIZE	256
+#define NEW_MODE	0222
+#define MASK		0777
+
+char *TCID = "statx02";
+int TST_TOTAL = 1;
+
+uid_t user_id;			/* eff. user id/group id of test process */
+gid_t group_id;
+char nobody_uid[] = "nobody";
+struct passwd *ltpuser;
+
+void setup();
+void cleanup();
+
+int main(int ac, char **av)
+{
+	struct statx statx_buf;
+	struct stat stat_buf;
+	int lc;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+
+		tst_count = 0;
+
+		/*
+		 * Call statx(2) to get the status of
+		 * specified 'file' into statx structure.
+		 */
+		TEST(statx(AT_FDCWD, TESTFILE, 0, STATX_BASIC_STATS, &statx_buf));
+		if (TEST_RETURN == -1) {
+			tst_resm(TFAIL,
+				 "statx(%s, &statx_buf) Failed, errno=%d : %s",
+				 TESTFILE, TEST_ERRNO, strerror(TEST_ERRNO));
+			goto next;
+		}
+
+		TEST(stat(TESTFILE, &stat_buf));
+		if (TEST_RETURN == -1) {
+			tst_resm(TFAIL,
+				 "stat(%s, &statx_buf) Failed, errno=%d : %s",
+				 TESTFILE, TEST_ERRNO, strerror(TEST_ERRNO));
+			goto next;
+		}
+
+		if (tst_cmp_statx(&statx_buf, &stat_buf) == -1)
+			goto next;
+
+		statx_buf.stx_mode &= ~S_IFREG;
+		/*
+		 * Verify the data returned by statx(2) against the
+		 * expected data.
+		 */
+		if ((statx_buf.stx_uid != user_id) ||
+		    (statx_buf.stx_gid != group_id) ||
+		    (statx_buf.stx_size != FILE_SIZE) ||
+		    ((statx_buf.stx_mode & MASK) != NEW_MODE)) {
+			tst_resm(TFAIL, "Functionality of "
+				 "statx(2) on '%s' Failed",
+				 TESTFILE);
+		} else {
+			tst_resm(TPASS, "Functionality of "
+				 "statx(2) on '%s' Successful",
+				 TESTFILE);
+		}
+
+	next:
+		tst_count++;	/* incr TEST_LOOP counter */
+	}
+
+	cleanup();
+	tst_exit();
+}
+
+/*
+ * void
+ * setup() -  Performs setup function for the test.
+ *  Creat a temporary directory and change directory to it.
+ *  Creat a testfile and write some data into it.
+ *  Modify the mode permissions of testfile such that test process
+ *  has read-only access to testfile.
+ */
+void setup(void)
+{
+	int i, fd;		/* counter, file handle for file */
+	char tst_buff[BUF_SIZE];	/* data buffer for file */
+	int wbytes;		/* no. of bytes written to file */
+	int write_len = 0;
+
+	tst_require_root();
+
+	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+
+	/* Switch to nobody user for correct error code collection */
+	ltpuser = getpwnam(nobody_uid);
+	if (setuid(ltpuser->pw_uid) == -1) {
+		tst_resm(TINFO, "setuid failed to "
+			 "to set the effective uid to %d", ltpuser->pw_uid);
+		perror("setuid");
+	}
+
+	/* Pause if that option was specified
+	 * TEST_PAUSE contains the code to fork the test with the -i option.
+	 * You want to make sure you do this before you create your temporary
+	 * directory.
+	 */
+	TEST_PAUSE;
+
+	tst_tmpdir();
+
+	if ((fd = open(TESTFILE, O_RDWR | O_CREAT, FILE_MODE)) == -1) {
+		tst_brkm(TBROK, cleanup,
+			 "open(%s, O_RDWR|O_CREAT, %#o) Failed, errno=%d : %s",
+			 TESTFILE, FILE_MODE, errno, strerror(errno));
+	}
+
+	/* Fill the test buffer with the known data */
+	for (i = 0; i < BUF_SIZE; i++) {
+		tst_buff[i] = 'a';
+	}
+
+	/* Write to the file 1k data from the buffer */
+	while (write_len < FILE_SIZE) {
+		if ((wbytes = write(fd, tst_buff, sizeof(tst_buff))) <= 0) {
+			tst_brkm(TBROK | TERRNO, cleanup, "write to %s failed",
+				 TESTFILE);
+		} else {
+			write_len += wbytes;
+		}
+	}
+
+	if (close(fd) == -1) {
+		tst_resm(TWARN | TERRNO, "closing %s failed", TESTFILE);
+	}
+
+	/* Modify mode permissions on the testfile */
+	if (chmod(TESTFILE, NEW_MODE) < 0) {
+		tst_brkm(TBROK | TERRNO, cleanup, "chmodding %s failed",
+			 TESTFILE);
+	}
+
+	/* Get the uid/gid of the process */
+	user_id = getuid();
+	group_id = getgid();
+
+}
+
+/*
+ * cleanup() - performs all ONE TIME cleanup for this test at
+ *	       completion or premature exit.
+ *  Remove the temporary directory and file created.
+ */
+void cleanup(void)
+{
+
+	tst_rmdir();
+}
diff --git a/testcases/kernel/syscalls/statx/statx03.c b/testcases/kernel/syscalls/statx/statx03.c
new file mode 100644
index 0000000..24f5b60
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx03.c
@@ -0,0 +1,388 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from stat03.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (c) International Business Machines  Corp., 2001
+ *
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program;  if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/*
+ * Test Name: statx03
+ *
+ * Test Description:
+ *   Verify that,
+ *   1) statx(2) returns -1 and sets errno to EACCES if search permission is
+ *      denied on a component of the path prefix.
+ *   2) statx(2) returns -1 and sets errno to ENOENT if the specified file
+ *	does not exists or empty string.
+ *   3) statx(2) returns -1 and sets errno to EFAULT if pathname points
+ *	outside user's accessible address space.
+ *   4) statx(2) returns -1 and sets errno to ENAMETOOLONG if the pathname
+ *	component is too long.
+ *   5) statx(2) returns -1 and sets errno to ENOTDIR if the directory
+ *	component in pathname is not a directory.
+ *
+ * Expected Result:
+ *  statx() should fail with return value -1 and set expected errno.
+ *
+ * Algorithm:
+ *  Setup:
+ *   Setup signal handling.
+ *   Create temporary directory.
+ *   Pause for SIGUSR1 if option specified.
+ *
+ *  Test:
+ *   Loop if the proper options are given.
+ *   Execute system call
+ *   Check return code, if system call failed (return=-1)
+ *   	if errno set == expected errno
+ *   		Issue sys call fails with expected return value and errno.
+ *   	Otherwise,
+ *		Issue sys call fails with unexpected errno.
+ *   Otherwise,
+ *	Issue sys call returns unexpected value.
+ *
+ *  Cleanup:
+ *   Print errno log and/or timing stats if options given
+ *   Delete the temporary directory(s)/file(s) created.
+ *
+ * Usage:  <for command-line>
+ *  statx03 [-c n] [-e] [-i n] [-I x] [-p x] [-t]
+ *	where,  -c n : Run n copies concurrently.
+ *		-e   : Turn on errno logging.
+ *		-i n : Execute test n times.
+ *		-I x : Execute test for x seconds.
+ *		-P x : Pause for x seconds between iterations.
+ *		-t   : Turn on syscall timing.
+ *
+ * Restrictions:
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <pwd.h>
+
+#include "test.h"
+#include "linux-statx.h"
+
+#define MODE_RWX	S_IRWXU | S_IRWXG | S_IRWXO
+#define FILE_MODE	S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
+#define DIR_TEMP	"testdir_1"
+#define TEST_FILE1	"testdir_1/tfile_1"
+#define TEST_FILE2	"t_file/tfile_2"
+
+int no_setup();
+int setup1();			/* setup function to test chmod for EACCES */
+int setup2();			/* setup function to test chmod for ENOTDIR */
+int longpath_setup();		/* setup function to test chmod for ENAMETOOLONG */
+char nobody_uid[] = "nobody";
+struct passwd *ltpuser;
+
+char Longpathname[PATH_MAX + 2];
+char High_address_node[64];
+
+struct test_case_t {		/* test case struct. to hold ref. test cond's */
+	char *pathname;
+	char *desc;
+	int exp_errno;
+	int (*setupfunc) ();
+} Test_cases[] = {
+	{
+	TEST_FILE1, "No Search permissions to process", EACCES, setup1},
+#if !defined(UCLINUX)
+	{
+	High_address_node, "Address beyond address space", EFAULT, no_setup},
+	{
+	(char *)-1, "Negative address", EFAULT, no_setup},
+#endif
+	{
+	Longpathname, "Pathname too long", ENAMETOOLONG, longpath_setup}, {
+	"", "Pathname is empty", ENOENT, no_setup}, {
+	TEST_FILE2, "Path contains regular file", ENOTDIR, setup2}, {
+	NULL, NULL, 0, no_setup}
+};
+
+char *TCID = "statx03";
+int TST_TOTAL = ARRAY_SIZE(Test_cases);
+
+char *bad_addr = 0;
+
+void setup();			/* Main setup function for the tests */
+void cleanup();			/* cleanup function for the test */
+
+int main(int ac, char **av)
+{
+	struct statx statx_buf;
+	struct stat stat_buf;
+	int lc;
+	char *file_name;	/* ptr. for file name whose mode is modified */
+	char *test_desc;	/* test specific error message */
+	int ind;		/* counter to test different test conditions */
+	int x_retval, x_errno;
+	int s_retval, s_errno;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+
+	/*
+	 * Invoke setup function to call individual test setup functions
+	 * to simulate test conditions.
+	 */
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+
+		tst_count = 0;
+
+		for (ind = 0; Test_cases[ind].desc != NULL; ind++) {
+			file_name = Test_cases[ind].pathname;
+			test_desc = Test_cases[ind].desc;
+
+#if !defined(UCLINUX)
+			if (file_name == High_address_node) {
+				file_name = (char *)get_high_address();
+			}
+#endif
+
+			/*
+			 * Call statx(2) to test different test conditions.
+			 * verify that it fails with -1 return value and
+			 * sets appropriate errno.
+			 */
+			TEST(statx(AT_FDCWD, file_name, 0, STATX_BASIC_STATS, &statx_buf));
+			x_retval = TEST_RETURN;
+			x_errno = TEST_ERRNO;
+			TEST(stat(file_name, &stat_buf));
+			s_retval = TEST_RETURN;
+			s_errno = TEST_ERRNO;
+
+			/* Check return code from statx(2) */
+			if (x_retval != s_retval) {
+				tst_resm(TFAIL,
+					 "statx(), %s, return %d inconsistent with"
+					 " stat() return %d, expected errno:%d",
+					 test_desc, x_retval, s_retval,
+					 Test_cases[ind].exp_errno);
+			} else if (x_retval == -1 && x_errno != s_errno) {
+				tst_resm(TFAIL,
+					 "statx(), %s, error %d inconsistent with"
+					 " stat() error %d, exp %d",
+					 test_desc, x_errno, s_errno,
+					 Test_cases[ind].exp_errno);
+			} else if (TEST_RETURN == -1) {
+				if (TEST_ERRNO == Test_cases[ind].exp_errno) {
+					tst_resm(TPASS,
+						 "statx() fails, %s, errno:%d",
+						 test_desc, TEST_ERRNO);
+				} else {
+					tst_resm(TFAIL,
+						 "statx() fails, %s, errno:%d, expected errno:%d",
+						 test_desc, TEST_ERRNO,
+						 Test_cases[ind].exp_errno);
+				}
+			} else if (tst_cmp_statx(&statx_buf, &stat_buf) == -1) {
+				;
+			} else {
+				tst_resm(TFAIL,
+					 "statx(2) returned %ld, expected -1, errno:%d",
+					 TEST_RETURN,
+					 Test_cases[ind].exp_errno);
+			}
+		}
+		tst_count++;	/* incr TEST_LOOP counter */
+	}
+
+	/*
+	 * Invoke cleanup() to delete the test directory/file(s) created
+	 * in the setup().
+	 */
+	cleanup();
+	tst_exit();
+
+}
+
+/*
+ * void
+ * setup(void) - performs all ONE TIME setup for this test.
+ * 	Exit the test program on receipt of unexpected signals.
+ *	Create a temporary directory and change directory to it.
+ *	Invoke individual test setup functions according to the order
+ *	set in struct. definition.
+ */
+void setup(void)
+{
+	int ind;
+
+	tst_require_root();
+
+	/* Capture unexpected signals */
+	tst_sig(FORK, DEF_HANDLER, cleanup);
+
+	/* Switch to nobody user for correct error code collection */
+	ltpuser = getpwnam(nobody_uid);
+	if (setuid(ltpuser->pw_uid) == -1) {
+		tst_resm(TINFO, "setuid failed to "
+			 "to set the effective uid to %d", ltpuser->pw_uid);
+		perror("setuid");
+	}
+
+	/* Pause if that option was specified
+	 * TEST_PAUSE contains the code to fork the test with the -i option.
+	 * You want to make sure you do this before you create your temporary
+	 * directory.
+	 */
+	TEST_PAUSE;
+
+	/* Make a temp dir and cd to it */
+	tst_tmpdir();
+
+#if !defined(UCLINUX)
+	bad_addr = mmap(0, 1, PROT_NONE,
+			MAP_PRIVATE_EXCEPT_UCLINUX | MAP_ANONYMOUS, 0, 0);
+	if (bad_addr == MAP_FAILED) {
+		tst_brkm(TBROK, cleanup, "mmap failed");
+	}
+	Test_cases[2].pathname = bad_addr;
+#endif
+
+	/* call individual setup functions */
+	for (ind = 0; Test_cases[ind].desc != NULL; ind++) {
+		Test_cases[ind].setupfunc();
+	}
+}
+
+/*
+ * int
+ * no_setup() - Some test conditions for statx(2) do not any setup.
+ *              Hence, this function just returns 0.
+ *  This function simply returns 0.
+ */
+int no_setup(void)
+{
+	return 0;
+}
+
+/*
+ * int
+ * setup1() - setup function for a test condition for which statx(2)
+ *	      returns -1 and sets errno to EACCES.
+ *  Create a test directory under temporary directory and create a test file
+ *  under this directory with mode "0666" permissions.
+ *  Modify the mode permissions on test directory such that process will not
+ *  have search permissions on test directory.
+ *
+ *  The function returns 0.
+ */
+int setup1(void)
+{
+	int fd;			/* file handle for testfile */
+
+	/* Creat a test directory */
+	if (mkdir(DIR_TEMP, MODE_RWX) < 0) {
+		tst_brkm(TBROK, cleanup, "mkdir(2) of %s failed", DIR_TEMP);
+	}
+
+	/* Creat a test file under above test directory */
+	if ((fd = open(TEST_FILE1, O_RDWR | O_CREAT, 0666)) == -1) {
+		tst_brkm(TBROK, cleanup,
+			 "open(%s, O_RDWR|O_CREAT, 0666) failed, errno=%d : %s",
+			 TEST_FILE1, errno, strerror(errno));
+	}
+	/* Close the test file */
+	if (close(fd) == -1) {
+		tst_brkm(TBROK, cleanup,
+			 "close(%s) Failed, errno=%d : %s",
+			 TEST_FILE1, errno, strerror(errno));
+	}
+
+	/* Modify mode permissions on test directory */
+	if (chmod(DIR_TEMP, FILE_MODE) < 0) {
+		tst_brkm(TBROK, cleanup, "chmod(2) of %s failed", DIR_TEMP);
+	}
+	return 0;
+}
+
+/*
+ * int
+ * setup2() - setup function for a test condition for which statx(2)
+ *	     returns -1 and sets errno to ENOTDIR.
+ *
+ *  Create a test file under temporary directory so that test tries to
+ *  change mode of a testfile "tfile_2" under "t_file" which happens to be
+ *  another regular file.
+ */
+int setup2(void)
+{
+	int fd;			/* File handle for test file */
+
+	/* Creat a test file under temporary directory */
+	if ((fd = open("t_file", O_RDWR | O_CREAT, MODE_RWX)) == -1) {
+		tst_brkm(TBROK, cleanup,
+			 "open(2) on t_file failed, errno=%d : %s",
+			 errno, strerror(errno));
+	}
+	/* Close the test file created above */
+	if (close(fd) == -1) {
+		tst_brkm(TBROK, cleanup,
+			 "close(t_file) Failed, errno=%d : %s",
+			 errno, strerror(errno));
+	}
+	return 0;
+}
+
+/*
+ * int
+ * longpath_setup() - setup to create a node with a name length exceeding
+ *                    the MAX. length of PATH_MAX.
+ *   This function retruns 0.
+ */
+int longpath_setup(void)
+{
+	int ind;		/* counter variable */
+
+	for (ind = 0; ind <= (PATH_MAX + 1); ind++) {
+		Longpathname[ind] = 'a';
+	}
+	return 0;
+}
+
+/*
+ * void
+ * cleanup() - Performs all ONE TIME cleanup for this test at
+ *             completion or premature exit.
+ *	Print test timing stats and errno log if test executed with options.
+ *	Remove temporary directory and sub-directories/files under it
+ *	created during setup().
+ *	Exit the test program with normal exit code.
+ */
+void cleanup(void)
+{
+
+	/* Restore mode permissions on test directory created in setup2() */
+	if (chmod(DIR_TEMP, MODE_RWX) < 0) {
+		tst_brkm(TFAIL, NULL, "chmod(2) of %s failed", DIR_TEMP);
+	}
+
+	tst_rmdir();
+}
diff --git a/testcases/kernel/syscalls/statx/statx05.c b/testcases/kernel/syscalls/statx/statx05.c
new file mode 100644
index 0000000..8726e48
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx05.c
@@ -0,0 +1,206 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from stat05.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * Further, this software is distributed without any warranty that it is
+ * free of the rightful claim of any third person regarding infringement
+ * or the like.  Any license provided herein, whether implied or
+ * otherwise, applies only to this software file.  Patent licenses, if
+ * any, provided herein do not apply to combinations of this program with
+ * other software, or any other product whatsoever.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Contact information: Silicon Graphics, Inc., 1600 Amphitheatre Pkwy,
+ * Mountain View, CA  94043, or:
+ *
+ * http://www.sgi.com
+ *
+ * For further information regarding this notice, see:
+ *
+ * http://oss.sgi.com/projects/GenInfo/NoticeExplan/
+ *
+ */
+
+/**********************************************************
+ *
+ *    TEST IDENTIFIER	: statx05
+ *
+ *    EXECUTED BY	: anyone
+ *
+ *    TEST TITLE	: Basic test for statx(2)
+ *
+ *    TEST CASE TOTAL	: 1
+ *
+ *    WALL CLOCK TIME	: 1
+ *
+ *    CPU TYPES		: ALL
+ *
+ *    TEST CASES
+ *
+ * 	1.) statx(2) returns...(See Description)
+ *
+ *    INPUT SPECIFICATIONS
+ * 	The standard options for system call tests are accepted.
+ *	(See the parse_opts(3) man page).
+ *
+ *    OUTPUT SPECIFICATIONS
+ *$
+ *    DURATION
+ * 	Terminates - with frequency and infinite modes.
+ *
+ *    SIGNALS
+ * 	Uses SIGUSR1 to pause before test if option set.
+ * 	(See the parse_opts(3) man page).
+ *
+ *    RESOURCES
+ * 	None
+ *
+ *    ENVIRONMENTAL NEEDS
+ *      No run-time environmental needs.
+ *
+ *    SPECIAL PROCEDURAL REQUIREMENTS
+ * 	None
+ *
+ *    INTERCASE DEPENDENCIES
+ * 	None
+ *
+ *    DETAILED DESCRIPTION
+ *	This is a Phase I test for the statx(2) system call.  It is intended
+ *	to provide a limited exposure of the system call, for now.  It
+ *	should/will be extended when full functional tests are written for
+ *	statx(2).
+ *
+ * 	Setup:
+ * 	  Setup signal handling.
+ *	  Pause for SIGUSR1 if option specified.
+ *
+ * 	Test:
+ *	 Loop if the proper options are given.
+ * 	  Execute system call
+ *	  Check return code, if system call failed (return=-1)
+ *		Log the errno and Issue a FAIL message.
+ *	  Otherwise, Issue a PASS message.
+ *
+ * 	Cleanup:
+ * 	  Print errno log and/or timing stats if options given
+ *
+ *
+ *#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#**/
+
+#include <sys/types.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include "test.h"
+#include "linux-statx.h"
+
+void setup();
+void cleanup();
+
+char *TCID = "statx05";
+int TST_TOTAL = 1;
+
+char fname[255];
+int fd;
+struct statx stxatter;
+struct stat statter;
+
+int main(int ac, char **av)
+{
+	int x_retval, x_errno;
+	int s_retval, s_errno;
+	int lc;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+
+		tst_count = 0;
+
+		/*
+		 * Call statx(2) with F_CLRALF argument on fname
+		 */
+		TEST(statx(AT_FDCWD, fname, 0, STATX_BASIC_STATS, &stxatter));
+		x_retval = TEST_RETURN;
+		x_errno = TEST_ERRNO;
+		TEST(stat(fname, &statter));
+		s_retval = TEST_RETURN;
+		s_errno = TEST_ERRNO;
+
+		/* check return code */
+		if (x_retval != s_retval) {
+			tst_resm(TFAIL,
+				 "statx() return %d inconsistent with"
+				 " stat() return %d",
+				 x_retval, s_retval);
+		} else if (x_retval == -1 && x_errno != s_errno) {
+			tst_resm(TFAIL,
+				 "statx() error %d inconsistent with"
+				 " stat() error %d",
+				 x_errno, s_errno);
+		} else if (TEST_RETURN == -1) {
+			tst_resm(TFAIL, "statx(%s, &statter) failed", fname);
+		} else if (tst_cmp_statx(&stxatter, &statter) == -1) {
+			;
+		} else {
+			tst_resm(TPASS,
+				 "statx(%s, &stxatter) returned %ld",
+				 fname, TEST_RETURN);
+		}
+
+	}
+
+	cleanup();
+	tst_exit();
+}
+
+/***************************************************************
+ * setup() - performs all ONE TIME setup for this test.
+ ***************************************************************/
+void setup(void)
+{
+
+	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+
+	TEST_PAUSE;
+
+	tst_tmpdir();
+
+	sprintf(fname, "tfile_%d", getpid());
+	if ((fd = open(fname, O_RDWR | O_CREAT, 0700)) == -1) {
+		tst_brkm(TBROK, cleanup,
+			 "open(%s, O_RDWR|O_CREAT,0700) Failed, errno=%d : %s",
+			 fname, errno, strerror(errno));
+	}
+
+	if (close(fd) == -1) {
+		tst_resm(TWARN | TERRNO, "close(%s) failed", fname);
+	}
+}
+
+/***************************************************************
+ * cleanup() - performs all ONE TIME cleanup for this test at
+ *		completion or premature exit.
+ ***************************************************************/
+void cleanup(void)
+{
+
+	tst_rmdir();
+}
diff --git a/testcases/kernel/syscalls/statx/statx06.c b/testcases/kernel/syscalls/statx/statx06.c
new file mode 100644
index 0000000..5f931fb
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx06.c
@@ -0,0 +1,355 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from stat06.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * Further, this software is distributed without any warranty that it is
+ * free of the rightful claim of any third person regarding infringement
+ * or the like.  Any license provided herein, whether implied or
+ * otherwise, applies only to this software file.  Patent licenses, if
+ * any, provided herein do not apply to combinations of this program with
+ * other software, or any other product whatsoever.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Contact information: Silicon Graphics, Inc., 1600 Amphitheatre Pkwy,
+ * Mountain View, CA  94043, or:
+ *
+ * http://www.sgi.com
+ *
+ * For further information regarding this notice, see:
+ *
+ * http://oss.sgi.com/projects/GenInfo/NoticeExplan/
+ *
+ */
+
+/**********************************************************
+ *
+ *    TEST IDENTIFIER	: statx06
+ *
+ *    EXECUTED BY	: anyone
+ *
+ *    TEST TITLE	: statx(2) negative path testcases
+ *
+ *    TEST CASE TOTAL	: 7
+ *
+ *    WALL CLOCK TIME	: 1
+ *
+ *    CPU TYPES		: ALL
+ *
+ *    TEST CASES
+ *
+ * 	1-7) See Testcases structure below.
+ *
+ *    INPUT SPECIFICATIONS
+ * 	The standard options for system call tests are accepted.
+ *	(See the parse_opts(3) man page).
+ *      -h  : print help and exit
+ *
+ *    OUTPUT SPECIFICATIONS
+ *$
+ *    DURATION
+ * 	Terminates - with frequency and infinite modes.
+ *
+ *    SIGNALS
+ * 	Uses SIGUSR1 to pause before test if option set.
+ * 	(See the parse_opts(3) man page).
+ *
+ *    RESOURCES
+ * 	None
+ *
+ *    ENVIRONMENTAL NEEDS
+ * 	The libcuts.a and libsys.a libraries must be included in
+ *	the compilation of this test.
+ *
+ *    SPECIAL PROCEDURAL REQUIREMENTS
+ * 	None
+ *
+ *    INTERCASE DEPENDENCIES
+ * 	None
+ *
+ *    DETAILED DESCRIPTION
+ *	This is a Phase I test for the statx(2) system call.  It is intended
+ *	to provide a limited exposure of the system call, for now.  It
+ *	should/will be extended when full functional tests are written for
+ *	statx(2).
+ *
+ * 	Setup:
+ * 	  Setup signal handling.
+ *	  Pause for SIGUSR1 if option specified.
+ *
+ * 	Test:
+ *	 Loop if the proper options are given.
+ * 	  Execute system call
+ *	  Check return code, if system call failed (return=-1)
+ *		Log the errno and Issue a FAIL message.
+ *	  Otherwise, Issue a PASS message.
+ *
+ * 	Cleanup:
+ * 	  Print errno log and/or timing stats if options given
+ *
+ *
+ *#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#**/
+
+#include <sys/types.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <setjmp.h>
+#include <unistd.h>
+#include "test.h"
+#include "linux-statx.h"
+
+void setup();
+void cleanup();
+
+char *TCID = "statx06";
+
+char *bad_addr = 0;
+
+#if !defined(UCLINUX)
+int high_address_setup();
+char High_address[64];
+#endif
+int longpath_setup();
+int no_setup();
+int filepath_setup();
+char Longpathname[PATH_MAX + 2];
+struct statx statxbuf;
+jmp_buf sig11_recover;
+void sig11_handler(int sig);
+
+struct test_case_t {
+	char *pathname;
+	struct statx *stxbuf;
+	char *desc;
+	int exp_errno;
+	int (*setupfunc) ();
+} Test_cases[] = {
+	{
+	"nonexistfile", &statxbuf, "non-existent file", ENOENT, no_setup}, {
+	"", &statxbuf, "path is empty string", ENOENT, no_setup}, {
+	"nefile/file", &statxbuf, "path contains a non-existent file",
+		    ENOENT, no_setup}, {
+	"file/file", &statxbuf, "path contains a regular file",
+		    ENOTDIR, filepath_setup}, {
+	Longpathname, &statxbuf, "pathname too long", ENAMETOOLONG,
+		    longpath_setup},
+#if !defined(UCLINUX)
+	{
+	High_address, &statxbuf, "address beyond address space", EFAULT,
+		    high_address_setup}, {
+	(char *)-1, &statxbuf, "negative address", EFAULT, no_setup},
+#endif
+	{
+	NULL, NULL, NULL, 0, no_setup}
+};
+
+int TST_TOTAL = ARRAY_SIZE(Test_cases);
+
+/***********************************************************************
+ * Main
+ ***********************************************************************/
+int main(int ac, char **av)
+{
+	int lc;
+	char *fname;
+	char *desc;
+	int ind;
+	struct statx *stxbuf;
+	struct sigaction sa, osa;
+
+    /***************************************************************
+     * parse standard options
+     ***************************************************************/
+	tst_parse_opts(ac, av, NULL, NULL);
+
+    /***************************************************************
+     * perform global setup for test
+     ***************************************************************/
+	setup();
+
+    /***************************************************************
+     * check looping state if -c option given
+     ***************************************************************/
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+
+		tst_count = 0;
+
+		for (ind = 0; Test_cases[ind].desc != NULL; ind++) {
+
+			fname = Test_cases[ind].pathname;
+			desc = Test_cases[ind].desc;
+			stxbuf = Test_cases[ind].stxbuf;
+
+			if (stxbuf == (struct statx *)-1L) {
+				/* special sig11 case */
+				sa.sa_handler = &sig11_handler;
+				sigemptyset(&sa.sa_mask);
+				sa.sa_flags = 0;
+
+				sigaction(SIGSEGV, NULL, &osa);
+				sigaction(SIGSEGV, &sa, NULL);
+
+				if (setjmp(sig11_recover)) {
+					TEST_RETURN = -1;
+					TEST_ERRNO = EFAULT;
+				} else {
+					TEST(statx(AT_FDCWD, fname, 0,
+						   STATX_BASIC_STATS, stxbuf));
+				}
+				sigaction(SIGSEGV, &osa, NULL);
+			} else {
+				/*
+				 *  Call statx(2)
+				 */
+
+				TEST(statx(AT_FDCWD, fname, 0,
+					   STATX_BASIC_STATS, stxbuf));
+			}
+
+			/* check return code */
+			if (TEST_RETURN == -1) {
+				if (TEST_ERRNO ==
+				    Test_cases[ind].exp_errno)
+					tst_resm(TPASS,
+						 "statx(<%s>, &stbuf) Failed, errno=%d",
+						 desc, TEST_ERRNO);
+				else
+					tst_resm(TFAIL,
+						 "statx(<%s>, &stbuf) Failed, errno=%d, expected errno:%d",
+						 desc, TEST_ERRNO,
+						 Test_cases
+						 [ind].exp_errno);
+			} else {
+				tst_resm(TFAIL,
+					 "statx(<%s>, &stbuf) returned %ld, expected -1, errno:%d",
+					 desc, TEST_RETURN,
+					 Test_cases[ind].exp_errno);
+			}
+		}
+
+	}
+
+	cleanup();
+	tst_exit();
+}
+
+/***************************************************************
+ * setup() - performs all ONE TIME setup for this test.
+ ***************************************************************/
+void setup(void)
+{
+	int ind;
+
+	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+
+	TEST_PAUSE;
+
+	tst_tmpdir();
+
+#if !defined(UCLINUX)
+	bad_addr = mmap(0, 1, PROT_NONE,
+			MAP_PRIVATE_EXCEPT_UCLINUX | MAP_ANONYMOUS, 0, 0);
+	if (bad_addr == MAP_FAILED) {
+		tst_brkm(TBROK, cleanup, "mmap failed");
+	}
+	Test_cases[6].pathname = bad_addr;
+#endif
+
+	for (ind = 0; Test_cases[ind].desc != NULL; ind++) {
+		Test_cases[ind].setupfunc();
+	}
+
+}
+
+/***************************************************************
+ * cleanup() - performs all ONE TIME cleanup for this test at
+ *		completion or premature exit.
+ ***************************************************************/
+void cleanup(void)
+{
+
+	tst_rmdir();
+
+}
+
+/******************************************************************
+ * no_setup() - does nothing
+ ******************************************************************/
+int no_setup(void)
+{
+	return 0;
+}
+
+#if !defined(UCLINUX)
+
+/******************************************************************
+ * high_address_setup() - generates an address that should cause a segfault
+ ******************************************************************/
+int high_address_setup(void)
+{
+	int ind;
+
+	for (ind = 0; Test_cases[ind].desc != NULL; ind++) {
+		if (Test_cases[ind].pathname == High_address) {
+			/*if (strcmp(Test_cases[ind].pathname, HIGH_ADDRESS) == 0) { ** */
+			Test_cases[ind].pathname = (char *)(sbrk(0) + 5);
+			break;
+		}
+	}
+	return 0;
+
+}
+#endif
+
+/******************************************************************
+ * longpath_setup() - creates a filename that is too long
+ ******************************************************************/
+int longpath_setup(void)
+{
+	int ind;
+
+	for (ind = 0; ind <= PATH_MAX + 1; ind++) {
+		Longpathname[ind] = 'a';
+	}
+	return 0;
+
+}
+
+/******************************************************************
+ * filepath_setup() creates a file the exists that we will treat as a directory
+ ******************************************************************/
+int filepath_setup(void)
+{
+	int fd;
+
+	if ((fd = creat("file", 0777)) == -1) {
+		tst_brkm(TBROK, cleanup, "creat(file) failed, errno:%d %s",
+			 errno, strerror(errno));
+	}
+	close(fd);
+	return 0;
+}
+
+/******************************************************************
+ * sig11_handler() - our segfault recover hack
+ ******************************************************************/
+void sig11_handler(int sig)
+{
+	longjmp(sig11_recover, 1);
+}
diff --git a/testcases/kernel/syscalls/statx/statx11.c b/testcases/kernel/syscalls/statx/statx11.c
new file mode 100644
index 0000000..cae9fb0
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx11.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from fstat01.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * Further, this software is distributed without any warranty that it is
+ * free of the rightful claim of any third person regarding infringement
+ * or the like.  Any license provided herein, whether implied or
+ * otherwise, applies only to this software file.  Patent licenses, if
+ * any, provided herein do not apply to combinations of this program with
+ * other software, or any other product whatsoever.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Contact information: Silicon Graphics, Inc., 1600 Amphitheatre Pkwy,
+ * Mountain View, CA  94043, or:
+ *
+ * http://www.sgi.com
+ *
+ * For further information regarding this notice, see:
+ *
+ * http://oss.sgi.com/projects/GenInfo/NoticeExplan/
+ *
+ */
+
+/**********************************************************
+ *
+ *    TEST IDENTIFIER	: statx11
+ *
+ *    EXECUTED BY	: anyone
+ *
+ *    TEST TITLE	: Basic test for statx(2) on fd
+ *
+ *    TEST CASE TOTAL	: 1
+ *
+ *    WALL CLOCK TIME	: 1
+ *
+ *    CPU TYPES		: ALL
+ *
+ *    TEST CASES
+ *
+ *	1.) statx(2) on fd returns...(See Description)
+ *
+ *    INPUT SPECIFICATIONS
+ *	The standard options for system call tests are accepted.
+ *	(See the parse_opts(3) man page).
+ *
+ *    OUTPUT SPECIFICATIONS
+ *
+ *    DURATION
+ *	Terminates - with frequency and infinite modes.
+ *
+ *    SIGNALS
+ *	Uses SIGUSR1 to pause before test if option set.
+ *	(See the parse_opts(3) man page).
+ *
+ *    RESOURCES
+ *	None
+ *
+ *    ENVIRONMENTAL NEEDS
+ *      No run-time environmental needs.
+ *
+ *    SPECIAL PROCEDURAL REQUIREMENTS
+ *	None
+ *
+ *    INTERCASE DEPENDENCIES
+ *	None
+ *
+ *    DETAILED DESCRIPTION
+ *	This is a Phase I test for the statx(2) system call.  It is intended
+ *	to provide a limited exposure of the system call, for now.  It
+ *	should/will be extended when full functional tests are written for
+ *	statx(2).
+ *
+ *	Setup:
+ *	  Setup signal handling.
+ *	  Pause for SIGUSR1 if option specified.
+ *
+ *	Test:
+ *	 Loop if the proper options are given.
+ *	  Execute system call
+ *	  Check return code, if system call failed (return=-1)
+ *		Log the errno and Issue a FAIL message.
+ *	  Otherwise, Issue a PASS message.
+ *
+ *	Cleanup:
+ *	  Print errno log and/or timing stats if options given
+ *
+ *
+ *#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#**/
+
+#include <sys/types.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include "test.h"
+#include "linux-statx.h"
+
+void setup();
+void cleanup();
+
+char *TCID = "statx11";
+int TST_TOTAL = 1;
+
+char fname[255];
+int fd;
+struct statx stxatter;
+struct stat statter;
+
+int main(int ac, char **av)
+{
+	int lc;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+
+		tst_count = 0;
+
+		TEST(statx(fd, NULL, 0, STATX_BASIC_STATS, &stxatter));
+		if (TEST_RETURN == -1) {
+			tst_resm(TFAIL | TTERRNO, "statx failed");
+			continue;
+		}
+
+		TEST(fstat(fd, &statter));
+		if (TEST_RETURN == -1) {
+			tst_resm(TFAIL | TTERRNO, "fstat failed");
+			continue;
+		}
+
+		if (tst_cmp_statx(&stxatter, &statter) == -1)
+			continue;
+
+		tst_resm(TPASS, "statx on fd returned %ld", TEST_RETURN);
+	}
+
+	cleanup();
+	tst_exit();
+}
+
+void setup(void)
+{
+
+	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+
+	TEST_PAUSE;
+
+	tst_tmpdir();
+
+	sprintf(fname, "tfile_%d", getpid());
+	fd = open(fname, O_RDWR | O_CREAT, 0700);
+	if (fd == -1)
+		tst_brkm(TBROK | TERRNO, cleanup, "open failed");
+}
+
+void cleanup(void)
+{
+	if (close(fd) == -1)
+		tst_resm(TWARN | TERRNO, "close(%s) failed", fname);
+
+	tst_rmdir();
+
+}
diff --git a/testcases/kernel/syscalls/statx/statx12.c b/testcases/kernel/syscalls/statx/statx12.c
new file mode 100644
index 0000000..06fd2a6
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx12.c
@@ -0,0 +1,164 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from fstat02.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (c) International Business Machines  Corp., 2001
+ *
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program;  if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+/*
+ * fstat() should return value 0 on success and the stat structure elements
+ * should be filled with specified 'file' information.
+ */
+#include <stdio.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <pwd.h>
+
+#include "test.h"
+#include "safe_macros.h"
+#include "linux-statx.h"
+
+#define FILE_MODE	0644
+#define TESTFILE	"testfile"
+#define FILE_SIZE       1024
+#define BUF_SIZE	256
+#define MASK		0777
+
+char *TCID = "statx12";
+int TST_TOTAL = 1;
+static uid_t user_id;
+static gid_t group_id;
+static int fildes;
+
+static void setup(void);
+static void cleanup(void);
+
+static void verify(void)
+{
+	struct statx statx_buf;
+	struct stat stat_buf;
+	int fail = 0;
+
+	TEST(statx(fildes, NULL, 0, STATX_BASIC_STATS, &statx_buf));
+	if (TEST_RETURN == -1) {
+		tst_resm(TFAIL | TTERRNO, "statx(%s) failed", TESTFILE);
+		return;
+	}
+
+	TEST(fstat(fildes, &stat_buf));
+	if (TEST_RETURN == -1) {
+		tst_resm(TFAIL | TTERRNO, "fstat(%s) failed", TESTFILE);
+		return;
+	}
+
+	if (tst_cmp_statx(&statx_buf, &stat_buf) == -1)
+		return;
+
+	if (statx_buf.stx_uid != user_id) {
+		tst_resm(TINFO, "statx_buf.stx_uid = %i expected %i",
+		         statx_buf.stx_uid, user_id);
+		fail++;
+	}
+
+	if (statx_buf.stx_gid != group_id) {
+		tst_resm(TINFO, "statx_buf.stx_gid = %i expected %i",
+		         statx_buf.stx_gid, group_id);
+		fail++;
+	}
+
+	if (statx_buf.stx_size != FILE_SIZE) {
+		tst_resm(TINFO, "statx_buf.stx_size = %llu expected %i",
+		         statx_buf.stx_size, FILE_SIZE);
+		fail++;
+	}
+
+        if ((statx_buf.stx_mode & MASK) != FILE_MODE) {
+		tst_resm(TINFO, "statx_buf.stx_mode = %o expected %o",
+		         (statx_buf.stx_mode & MASK), FILE_MODE);
+		fail++;
+	}
+
+	if (fail) {
+		tst_resm(TFAIL, "functionality of statx on fd incorrect");
+		return;
+	}
+
+	tst_resm(TPASS, "functionality of statx on fd correct");
+}
+
+int main(int ac, char **av)
+{
+	int lc;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); lc++)
+		verify();
+
+	cleanup();
+	tst_exit();
+}
+
+static void setup(void)
+{
+	struct passwd *ltpuser;
+	char tst_buff[BUF_SIZE];
+	int wbytes;
+	int write_len = 0;
+
+	tst_require_root();
+
+	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+
+	ltpuser = SAFE_GETPWNAM(NULL, "nobody");
+	SAFE_SETUID(NULL, ltpuser->pw_uid);
+
+	TEST_PAUSE;
+
+	tst_tmpdir();
+
+	umask(022);
+
+	fildes = SAFE_OPEN(tst_rmdir, TESTFILE, O_WRONLY | O_CREAT, FILE_MODE);
+
+	/* Fill the test buffer with the known data */
+	memset(tst_buff, 'a', BUF_SIZE - 1);
+
+	/* Write to the file 1k data from the buffer */
+	while (write_len < FILE_SIZE) {
+		if ((wbytes = write(fildes, tst_buff, sizeof(tst_buff))) <= 0)
+			tst_brkm(TBROK | TERRNO, cleanup, "write failed");
+		else
+			write_len += wbytes;
+	}
+
+	user_id = getuid();
+	group_id = getgid();
+}
+
+static void cleanup(void)
+{
+	if (close(fildes) == -1)
+		tst_resm(TWARN | TERRNO, "close failed");
+
+	tst_rmdir();
+}
diff --git a/testcases/kernel/syscalls/statx/statx13.c b/testcases/kernel/syscalls/statx/statx13.c
new file mode 100644
index 0000000..cc45c3b
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx13.c
@@ -0,0 +1,163 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from fstat03.c by David Howells (dhowells@redhat.com)
+ *
+ * Test Name: statx13
+ *
+ * Test Description:
+ *   Verify that, statx(2) returns -1 and sets errno to EBADF if the file
+ *   pointed to by file descriptor is not valid.
+ *
+ * Expected Result:
+ *  xstat() should fail with return value -1 and set expected errno.
+ *
+ * Algorithm:
+ *  Setup:
+ *   Setup signal handling.
+ *   Create temporary directory.
+ *   Pause for SIGUSR1 if option specified.
+ *
+ *  Test:
+ *   Loop if the proper options are given.
+ *   Execute system call
+ *   Check return code, if system call failed (return=-1)
+ *	if errno set == expected errno
+ *		Issue sys call fails with expected return value and errno.
+ *	Otherwise,
+ *		Issue sys call fails with unexpected errno.
+ *   Otherwise,
+ *	Issue sys call returns unexpected value.
+ *
+ *  Cleanup:
+ *   Print errno log and/or timing stats if options given
+ *   Delete the temporary directory(s)/file(s) created.
+ *
+ * Usage:  <for command-line>
+ *  statx13 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
+ *     where,  -c n : Run n copies concurrently.
+ *             -e   : Turn on errno logging.
+ *	       -i n : Execute test n times.
+ *	       -I x : Execute test for x seconds.
+ *	       -P x : Pause for x seconds between iterations.
+ *	       -t   : Turn on syscall timing.
+ *
+ * RESTRICTIONS:
+ *  This test should be executed by 'non-super-user' only.
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include "test.h"
+#include "linux-statx.h"
+
+#define FILE_MODE	S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
+#define TEST_FILE	"testfile"
+
+char *TCID = "statx13";
+int TST_TOTAL = 1;
+
+int fildes;			/* testfile descriptor */
+
+void setup();			/* Main setup function for the tests */
+void cleanup();			/* cleanup function for the test */
+
+int main(int ac, char **av)
+{
+	struct statx statx_buf;
+	int lc;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+
+	/*
+	 * Invoke setup function to create a testfile under temporary
+	 * directory.
+	 */
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+
+		tst_count = 0;
+		/*
+		 * Call xstat(2) to get the status information
+		 * of a closed testfile pointed to by 'fd'.
+		 * verify that fstat fails with -1 return value and
+		 * sets appropriate errno.
+		 */
+		TEST(statx(fildes, NULL, 0, STATX_BASIC_STATS, &statx_buf));
+
+		/* Check return code from fstat(2) */
+		if (TEST_RETURN == -1) {
+			if (TEST_ERRNO == EBADF) {
+				tst_resm(TPASS,
+					 "statx() on fd fails with expected error EBADF");
+			} else {
+				tst_resm(TFAIL | TTERRNO,
+					 "statx() on fd did not fail with EBADF");
+			}
+		} else {
+			tst_resm(TFAIL, "statx() on fd returned %ld, expected -1",
+				 TEST_RETURN);
+		}
+	}
+
+	/*
+	 * Invoke cleanup() to delete the test directory/file(s) created
+	 * in the setup().
+	 */
+	cleanup();
+
+	tst_exit();
+}
+
+/*
+ * void
+ * setup(void) - performs all ONE TIME setup for this test.
+ *	Exit the test program on receipt of unexpected signals.
+ *	Create a temporary directory and change directory to it.
+ *      Create a testfile under temporary directory.
+ *      Close the testfile.
+ */
+void setup(void)
+{
+	/* Capture unexpected signals */
+	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+
+	TEST_PAUSE;
+
+	/* Make a temp dir and cd to it */
+	tst_tmpdir();
+
+	/* Create a testfile under temporary directory */
+	fildes = open(TEST_FILE, O_RDWR | O_CREAT, 0666);
+	if (fildes == -1)
+		tst_brkm(TBROK | TERRNO, cleanup,
+			 "open(%s, O_RDWR|O_CREAT, 0666) failed", TEST_FILE);
+
+	if (close(fildes) == -1)
+		tst_brkm(TBROK | TERRNO, cleanup, "close(%s) failed",
+			 TEST_FILE);
+}
+
+/*
+ * void
+ * cleanup() - Performs all ONE TIME cleanup for this test at
+ *             completion or premature exit.
+ *	Print test timing stats and errno log if test executed with options.
+ *	Close the testfile if still opened.
+ *	Remove temporary directory and sub-directories/files under it
+ *	created during setup().
+ *	Exit the test program with normal exit code.
+ */
+void cleanup(void)
+{
+	tst_rmdir();
+}
diff --git a/testcases/kernel/syscalls/statx/statx15.c b/testcases/kernel/syscalls/statx/statx15.c
new file mode 100644
index 0000000..8505ef3
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx15.c
@@ -0,0 +1,248 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from fstat05.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (C) Bull S.A. 2001
+ * Copyright (c) International Business Machines  Corp., 2001
+ *
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program;  if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/*
+ * Test Name: statx15
+ *
+ * Test Description:
+ *   Verify that,
+ *   if buffer points outside user's accessible address space statx(2)
+ *	either returns -1 and sets errno to EFAULT
+ *	or SIGSEGV is returned instead of EFAULT
+ *
+ * Expected Result:
+ *   statx() should fail with return value -1 and set expected errno.
+ *    or
+ *   statx() should fail with SIGSEGV returned.
+ *   Both results are considered as acceptable.
+ *
+ * Algorithm:
+ *  Setup:
+ *   Setup signal handling SIGSEGV included.
+ *   Switch to nobody user.
+ *   Pause for SIGUSR1 if option specified.
+ *   Create temporary directory.
+ *   Create a testfile under temporary directory.
+ *
+ *  Test:
+ *   Buffer points outside user's accessible address space.
+ *   Loop if the proper options are given.
+ *   Execute system call
+ *   Check return code, if system call failed (return=-1)
+ *	if errno set == expected errno
+ *		Issue sys call fails with expected return value and errno.
+ *	Otherwise,
+ *		Issue sys call fails with unexpected errno.
+ *   Otherwise,
+ *	Issue sys call returns unexpected value.
+ *
+ *  Sighandler:
+ *	if signal == SIGSEGV
+ *		Issue sys call fails with expected signal
+ *      Otherwise,
+ *              Issue sys call fails with unexpected signal.
+ *
+ *  Cleanup:
+ *   Print errno log and/or timing stats if options given
+ *   Close the test file
+ *   Delete the temporary directory(s)/file(s) created.
+ *
+ * Usage:  <for command-line>
+ *  statx15 [-c n] [-e] [-i n] [-I x] [-p x] [-t]
+ *	where,  -c n : Run n copies concurrently.
+ *		-e   : Turn on errno logging.
+ *		-i n : Execute test n times.
+ *		-I x : Execute test for x seconds.
+ *		-P x : Pause for x seconds between iterations.
+ *		-t   : Turn on syscall timing.
+ *
+ * Restrictions:
+ *      This test must be run as root.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <pwd.h>
+
+#include "test.h"
+#include "linux-statx.h"
+
+#define TEST_FILE       "testfile"
+
+char nobody_uid[] = "nobody";
+struct passwd *ltpuser;
+
+char *TCID = "statx15";
+int TST_TOTAL = 1;
+
+int fildes;			/* testfile descriptor */
+
+void setup();			/* Main setup function for the tests */
+void cleanup();			/* cleanup function for the test */
+void sighandler(int sig);	/* signals handler function for the test */
+
+int siglist[] = { SIGHUP, SIGINT, SIGQUIT, SIGILL, SIGTRAP, SIGABRT, SIGIOT,
+	SIGBUS, SIGFPE, SIGUSR1, SIGSEGV, SIGUSR2, SIGPIPE, SIGALRM,
+	SIGTERM,
+#ifdef SIGSTKFLT
+	SIGSTKFLT,
+#endif
+	SIGCHLD, SIGCONT, SIGTSTP, SIGTTIN,
+	SIGTTOU, SIGURG, SIGXCPU, SIGXFSZ, SIGVTALRM, SIGPROF,
+	SIGWINCH, SIGIO, SIGPWR, SIGSYS,
+#ifdef SIGUNUSED
+	SIGUNUSED
+#endif
+};
+
+int SIG_SEEN = sizeof(siglist) / sizeof(int);
+
+#if !defined(UCLINUX)
+
+int main(int ac, char **av)
+{
+	struct statx statx_buf;
+	struct statx *ptr_str;
+	int lc;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+
+	/* Buffer points outside user's accessible address space. */
+	ptr_str = &statx_buf;	/* if it was for conformance testing */
+	ptr_str = (void *)sbrk(0) + (4 * getpagesize());
+
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+
+		tst_count = 0;
+
+		/* Call statx(2).
+		 * verify that it fails with -1 return value and
+		 * sets appropriate errno.
+		 */
+		TEST(statx(fildes, NULL, 0, STATX_BASIC_STATS, ptr_str));
+
+		/* Check return code from statx(2) */
+		if (TEST_RETURN == -1) {
+			if (TEST_ERRNO == EFAULT)
+				tst_resm(TPASS,
+					 "statx failed with EFAULT as expected");
+			else
+				tst_resm(TFAIL | TTERRNO,
+					 "statx failed unexpectedly");
+		} else {
+			tst_resm(TFAIL, "statx() returned %ld but we wanted -1",
+				 TEST_RETURN);
+		}
+	}
+
+	/*
+	 * Invoke cleanup() to delete the test directory/file(s) created
+	 * in the setup().
+	 */
+	cleanup();
+	tst_exit();
+}
+
+#else
+
+int main(void)
+{
+	tst_brkm(TCONF, NULL, "test is not available on uClinux");
+}
+
+#endif /* if !defined(UCLINUX) */
+
+/*
+ * void
+ * setup(void) - performs all ONE TIME setup for this test.
+ *	Exit the test program on receipt of unexpected signals.
+ *	Create a temporary directory and change directory to it.
+ */
+void setup(void)
+{
+	int i;
+
+	tst_require_root();
+
+	/*
+	 * Capture unexpected signals SIGSEGV included
+	 * SIGSEGV being considered as acceptable as returned value
+	 */
+	for (i = 0; i < SIG_SEEN; i++)
+		signal(siglist[i], &sighandler);
+
+	ltpuser = getpwnam(nobody_uid);
+	if (setuid(ltpuser->pw_uid) == -1)
+		tst_brkm(TBROK | TERRNO, NULL, "setuid(%d) failed",
+			 ltpuser->pw_uid);
+
+	tst_tmpdir();
+
+	/* Create a testfile under temporary directory */
+	fildes = open(TEST_FILE, O_RDWR | O_CREAT, 0666);
+	if (fildes == -1)
+		tst_brkm(TBROK | TERRNO, cleanup,
+			 "open(%s, O_RDWR|O_CREAT, 0666) failed", TEST_FILE);
+
+	TEST_PAUSE;
+}
+
+/*
+ * void
+ * cleanup() - Performs all ONE TIME cleanup for this test at
+ *             completion or premature exit.
+ *	Print test timing stats and errno log if test executed with options.
+ *	Remove temporary directory and sub-directories/files under it
+ *	created during setup().
+ *	Exit the test program with normal exit code.
+ */
+void cleanup(void)
+{
+	if (close(fildes) == -1)
+		tst_brkm(TBROK | TERRNO, cleanup, "close(%s) failed",
+			 TEST_FILE);
+
+	tst_rmdir();
+}
+
+/*
+ * sighandler() - handle the signals
+ */
+
+void sighandler(int sig)
+{
+	if (sig == SIGSEGV)
+		tst_resm(TPASS, "statx failed as expected with SIGSEGV");
+	else
+		tst_brkm(TBROK, NULL, "Unexpected signal %d received.", sig);
+	cleanup();
+	tst_exit();
+}
diff --git a/testcases/kernel/syscalls/statx/statx21.c b/testcases/kernel/syscalls/statx/statx21.c
new file mode 100644
index 0000000..ebd43c6
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx21.c
@@ -0,0 +1,215 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from lstat01.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * Further, this software is distributed without any warranty that it is
+ * free of the rightful claim of any third person regarding infringement
+ * or the like.  Any license provided herein, whether implied or
+ * otherwise, applies only to this software file.  Patent licenses, if
+ * any, provided herein do not apply to combinations of this program with
+ * other software, or any other product whatsoever.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Contact information: Silicon Graphics, Inc., 1600 Amphitheatre Pkwy,
+ * Mountain View, CA  94043, or:
+ *
+ * http://www.sgi.com
+ *
+ * For further information regarding this notice, see:
+ *
+ * http://oss.sgi.com/projects/GenInfo/NoticeExplan/
+ *
+ */
+
+/**********************************************************
+ *
+ *    TEST IDENTIFIER	: statx21
+ *
+ *    EXECUTED BY	: anyone
+ *
+ *    TEST TITLE	: Basic test for statx(2)
+ *
+ *    TEST CASE TOTAL	: 1
+ *
+ *    WALL CLOCK TIME	: 1
+ *
+ *    CPU TYPES		: ALL
+ *
+ *    TEST CASES
+ *
+ * 	1.) statx(2) returns...(See Description)
+ *
+ *    INPUT SPECIFICATIONS
+ * 	The standard options for system call tests are accepted.
+ *	(See the parse_opts(3) man page).
+ *
+ *    OUTPUT SPECIFICATIONS
+ *$
+ *    DURATION
+ * 	Terminates - with frequency and infinite modes.
+ *
+ *    SIGNALS
+ * 	Uses SIGUSR1 to pause before test if option set.
+ * 	(See the parse_opts(3) man page).
+ *
+ *    RESOURCES
+ * 	None
+ *
+ *    ENVIRONMENTAL NEEDS
+ *      No run-time environmental needs.
+ *
+ *    SPECIAL PROCEDURAL REQUIREMENTS
+ * 	None
+ *
+ *    INTERCASE DEPENDENCIES
+ * 	None
+ *
+ *    DETAILED DESCRIPTION
+ *	This is a Phase I test for the statx(2) system call.  It is intended
+ *	to provide a limited exposure of the system call, for now.  It
+ *	should/will be extended when full functional tests are written for
+ *	statx(2).
+ *
+ * 	Setup:
+ * 	  Setup signal handling.
+ *	  Pause for SIGUSR1 if option specified.
+ *
+ * 	Test:
+ *	 Loop if the proper options are given.
+ * 	  Execute system call
+ *	  Check return code, if system call failed (return=-1)
+ *		Log the errno and Issue a FAIL message.
+ *	  Otherwise, Issue a PASS message.
+ *
+ * 	Cleanup:
+ * 	  Print errno log and/or timing stats if options given
+ *
+ *
+ *#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#**/
+
+#include <sys/types.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <fcntl.h>
+#include "test.h"
+#include "linux-statx.h"
+
+void setup();
+void cleanup();
+
+char *TCID = "statx21";
+int TST_TOTAL = 1;
+
+char fname[255], lname[255], symlnk[255], buf[255];
+int fd;
+struct statx stxatter;
+struct stat statter;
+
+int main(int ac, char **av)
+{
+	int lc;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+	setup();
+
+	/***************************************************************
+	 * check looping state if -c option given
+	 ***************************************************************/
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+
+		tst_count = 0;
+
+		/*
+		 *  Call statx(2)
+		 */
+		TEST(statx(AT_FDCWD, symlnk, AT_SYMLINK_NOFOLLOW,
+			   STATX_BASIC_STATS, &stxatter));
+		if (TEST_RETURN == -1) {
+			tst_resm(TFAIL,
+				 "statx(%s, &statter) Failed, errno=%d : %s",
+				 symlnk, TEST_ERRNO, strerror(TEST_ERRNO));
+			continue;
+		}
+
+		TEST(lstat(symlnk, &statter));
+		if (TEST_RETURN == -1) {
+			tst_resm(TFAIL,
+				 "lstat(%s, &statter) Failed, errno=%d : %s",
+				 symlnk, TEST_ERRNO, strerror(TEST_ERRNO));
+			continue;
+		}
+
+		if (tst_cmp_statx(&stxatter, &statter) == -1)
+			continue;
+
+		tst_resm(TPASS,
+			 "statx(%s, &statter) returned %ld",
+			 symlnk, TEST_RETURN);
+	}
+
+	cleanup();
+	tst_exit();
+}
+
+/***************************************************************
+ * setup() - performs all ONE TIME setup for this test.
+ ***************************************************************/
+void setup(void)
+{
+
+	tst_sig(FORK, DEF_HANDLER, cleanup);
+
+	TEST_PAUSE;
+
+	tst_tmpdir();
+
+	sprintf(fname, "tfile_%d", getpid());
+	if ((fd = open(fname, O_RDWR | O_CREAT, 0700)) == -1) {
+		tst_brkm(TBROK, cleanup,
+			 "open(%s, O_RDWR|O_CREAT,0700) Failed, errno=%d : %s",
+			 fname, errno, strerror(errno));
+	} else if (close(fd) == -1) {
+		tst_resm(TWARN, "close(%s) Failed, errno=%d : %s",
+			 fname, errno, strerror(errno));
+	}
+
+	sprintf(symlnk, "lnfile_%d", getpid());
+	if (symlink(fname, symlnk) == -1) {
+		tst_resm(TWARN, "symlink(%s, %s) Failed, errno=%d : %s",
+			 fname, symlnk, errno, strerror(errno));
+	} else if (readlink(symlnk, buf, 255) == -1) {
+		tst_resm(TWARN, "readlink(%s, buf, 255) Failed, errno=%d : %s",
+			 symlnk, errno, strerror(errno));
+	} else if (strcmp(buf, fname) != 0) {
+		tst_resm(TWARN,
+			 "Setup Failure : Expected symbolic link contents of %s : Actual is %s",
+			 fname, buf);
+	}
+}
+
+/***************************************************************
+ * cleanup() - performs all ONE TIME cleanup for this test at
+ *		completion or premature exit.
+ ***************************************************************/
+void cleanup(void)
+{
+
+	tst_rmdir();
+
+}
diff --git a/testcases/kernel/syscalls/statx/statx22.c b/testcases/kernel/syscalls/statx/statx22.c
new file mode 100644
index 0000000..93eb454
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx22.c
@@ -0,0 +1,192 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from lstat02.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (c) International Business Machines  Corp., 2001
+ *
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program;  if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+/*
+ * Test Description:
+ *   Verify that,
+ *   1) statx(2) returns -1 and sets errno to EACCES if search permission is
+ *	denied on a component of the path prefix.
+ *   2) statx(2) returns -1 and sets errno to ENOENT if the specified file
+ *	does not exists or empty string.
+ *   3) statx(2) returns -1 and sets errno to EFAULT if pathname points
+ *	outside user's accessible address space.
+ *   4) statx(2) returns -1 and sets errno to ENAMETOOLONG if the pathname
+ *	component is too long.
+ *   5) statx(2) returns -1 and sets errno to ENOTDIR if the directory
+ *	component in pathname is not a directory.
+ *   6) statx(2) returns -1 and sets errno to ELOOP if the pathname has too
+ *	many symbolic links encountered while traversing.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <pwd.h>
+
+#include "test.h"
+#include "safe_macros.h"
+#include "linux-statx.h"
+
+#define MODE_RWX	S_IRWXU | S_IRWXG | S_IRWXO
+#define FILE_MODE	S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
+#define TEST_DIR	"test_dir"
+#define TEST_EACCES	TEST_DIR"/test_eacces"
+#define TEST_ENOENT	""
+#define TEST_ENOTDIR	"test_file/test_enotdir"
+#define TEST_ELOOP	"/test_eloop"
+
+static char longpathname[PATH_MAX + 2];
+static char elooppathname[sizeof(TEST_ELOOP) * 43] = ".";
+
+#if !defined(UCLINUX)
+static void bad_addr_setup(int);
+static void high_address_setup(int);
+#endif
+
+static struct test_case_t {
+	char *pathname;
+	int exp_errno;
+	void (*setup) ();
+} test_cases[] = {
+	{TEST_EACCES, EACCES, NULL},
+	{TEST_ENOENT, ENOENT, NULL},
+#if !defined(UCLINUX)
+	{NULL, EFAULT, bad_addr_setup},
+	{NULL, EFAULT, high_address_setup},
+#endif
+	{longpathname, ENAMETOOLONG, NULL},
+	{TEST_ENOTDIR, ENOTDIR, NULL},
+	{elooppathname, ELOOP, NULL},
+};
+
+char *TCID = "statx22";
+int TST_TOTAL = ARRAY_SIZE(test_cases);
+
+static void setup(void);
+static void statx_verify(int);
+static void cleanup(void);
+
+int main(int ac, char **av)
+{
+	int lc;
+	int i;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+		tst_count = 0;
+		for (i = 0; i < TST_TOTAL; i++)
+			statx_verify(i);
+	}
+
+	cleanup();
+	tst_exit();
+}
+
+static void setup(void)
+{
+	int i;
+	struct passwd *ltpuser;
+
+	tst_require_root();
+
+	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+
+	ltpuser = SAFE_GETPWNAM(cleanup, "nobody");
+	SAFE_SETEUID(cleanup, ltpuser->pw_uid);
+
+	TEST_PAUSE;
+
+	tst_tmpdir();
+
+	SAFE_MKDIR(cleanup, TEST_DIR, MODE_RWX);
+	SAFE_TOUCH(cleanup, TEST_EACCES, 0666, NULL);
+	if (chmod(TEST_DIR, FILE_MODE) < 0)
+		tst_brkm(TBROK, cleanup, "chmod(2) of %s failed", TEST_DIR);
+
+	SAFE_TOUCH(cleanup, "test_file", MODE_RWX, NULL);
+
+	memset(longpathname, 'a', PATH_MAX+1);
+
+	SAFE_MKDIR(cleanup, "test_eloop", MODE_RWX);
+	SAFE_SYMLINK(cleanup, "../test_eloop", "test_eloop/test_eloop");
+	/*
+	 * NOTE: the ELOOP test is written based on that the consecutive
+	 * symlinks limits in kernel is hardwired to 40.
+	 */
+	for (i = 0; i < 43; i++)
+		strcat(elooppathname, TEST_ELOOP);
+}
+
+#if !defined(UCLINUX)
+static void bad_addr_setup(int i)
+{
+	test_cases[i].pathname = SAFE_MMAP(cleanup, 0, 1, PROT_NONE,
+					   MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+}
+
+static void high_address_setup(int i)
+{
+	test_cases[i].pathname = (char *)get_high_address();
+}
+#endif
+
+static void statx_verify(int i)
+{
+	struct statx statx_buf;
+
+	if (test_cases[i].setup != NULL)
+		test_cases[i].setup(i);
+
+	TEST(statx(AT_FDCWD, test_cases[i].pathname, AT_SYMLINK_NOFOLLOW,
+		   STATX_BASIC_STATS, &statx_buf));
+
+	if (TEST_RETURN != -1) {
+		tst_resm(TFAIL, "statx() returned %ld, expected -1, errno=%d",
+			 TEST_RETURN, test_cases[i].exp_errno);
+		return;
+	}
+
+	if (TEST_ERRNO == test_cases[i].exp_errno) {
+		tst_resm(TPASS | TTERRNO, "statx() failed as expected");
+	} else {
+		tst_resm(TFAIL | TTERRNO,
+			 "statx() failed unexpectedly; expected: %d - %s",
+			 test_cases[i].exp_errno,
+			 strerror(test_cases[i].exp_errno));
+	}
+}
+
+static void cleanup(void)
+{
+	if (seteuid(0))
+		tst_resm(TINFO | TERRNO, "Failet to seteuid(0) before cleanup");
+
+	tst_rmdir();
+}
diff --git a/testcases/kernel/syscalls/statx/statx23.c b/testcases/kernel/syscalls/statx/statx23.c
new file mode 100644
index 0000000..9256d14
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx23.c
@@ -0,0 +1,136 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Derived from lstat03.c by David Howells (dhowells@redhat.com)
+ *
+ * Copyright (c) International Business Machines  Corp., 2001
+ *
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program;  if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/*
+ * Test Name: statx03
+ *
+ * Test Description:
+ *  Verify that, statx(2) succeeds to get the status of a file pointed to by
+ *  symlink and fills the stat structure elements.
+ *
+ * Expected Result:
+ *  statx() should return value 0 on success and the stat structure elements
+ *  should be filled with the symlink file information.
+ */
+#include <stdio.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <pwd.h>
+
+#include "test.h"
+#include "safe_macros.h"
+#include "linux-statx.h"
+
+#define FILE_MODE	S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
+#define TESTFILE	"testfile"
+#define SFILE		"sfile"
+#define FILE_SIZE       1024
+#define BUF_SIZE	256
+#define PERMS		0644
+
+char *TCID = "statx23";
+int TST_TOTAL = 1;
+static uid_t user_id;
+static gid_t group_id;
+
+static char nobody_uid[] = "nobody";
+static struct passwd *ltpuser;
+
+static void setup(void);
+static void cleanup(void);
+
+int main(int ac, char **av)
+{
+	struct statx statx_buf;
+	struct stat stat_buf;
+	int lc;
+
+	tst_parse_opts(ac, av, NULL, NULL);
+
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+		tst_count = 0;
+
+		TEST(statx(AT_FDCWD, SFILE, AT_SYMLINK_NOFOLLOW,
+			   STATX_BASIC_STATS, &statx_buf));
+
+		if (TEST_RETURN == -1) {
+			tst_resm(TFAIL | TTERRNO, "statx(%s) failed", SFILE);
+			continue;
+		}
+
+		TEST(lstat(SFILE, &stat_buf));
+
+		if (TEST_RETURN == -1) {
+			tst_resm(TFAIL | TTERRNO, "lstat(%s) failed", SFILE);
+			continue;
+		}
+
+		if (tst_cmp_statx(&statx_buf, &stat_buf) == -1)
+			continue;
+
+		if ((statx_buf.stx_uid != user_id) ||
+		    (statx_buf.stx_gid != group_id) ||
+		    ((statx_buf.stx_mode & S_IFMT) != S_IFLNK) ||
+		    (statx_buf.stx_size != strlen(TESTFILE))) {
+			tst_resm(TFAIL, "Functionality of statx(2) on "
+				 "'%s' Failed", SFILE);
+		} else {
+			tst_resm(TPASS, "Functionality of statx(2) on "
+				 "'%s' Succcessful", SFILE);
+		}
+	}
+
+	cleanup();
+	tst_exit();
+}
+
+static void setup(void)
+{
+	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+
+	tst_require_root();
+
+	ltpuser = SAFE_GETPWNAM(NULL, nobody_uid);
+	SAFE_SETUID(NULL, ltpuser->pw_uid);
+
+	TEST_PAUSE;
+
+	tst_tmpdir();
+
+	if (tst_fill_file(TESTFILE, 'a', 1024, 1))
+		tst_brkm(TBROK, cleanup, "Failed to create " TESTFILE);
+
+	SAFE_SYMLINK(cleanup, TESTFILE, SFILE);
+
+	user_id = getuid();
+	group_id = getgid();
+}
+
+static void cleanup(void)
+{
+	tst_rmdir();
+}

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add initial batch of statx() LTP tests
  2017-03-28  0:13         ` [PATCH] Add initial batch of statx() LTP tests David Howells
@ 2017-03-28  6:28           ` Christoph Hellwig
  2017-03-28  8:23           ` David Howells
  1 sibling, 0 replies; 38+ messages in thread
From: Christoph Hellwig @ 2017-03-28  6:28 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, Darrick J. Wong, Andreas Dilger, Eric Biggers,
	Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

On Tue, Mar 28, 2017 at 01:13:27AM +0100, David Howells wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > > That means a filesystem can't simply return non-basic data unconditionally if
> > > possible.  I prefer letting it do so if it doesn't incur any extra I/O
> > > overheads.
> > 
> > This seems like it will lead to userspace expecting certain fields to
> > just be there, and a lot harder to properly verify for tests.  Which btw
> > we all need for these odd behaviors.  If we can't get them any time soon
> > (e.g. before -rc6) I think we'll simply have to revert statx instead of
> > leaving this untested mess in the tree.
> 
> Here you go.  First batch of tests.  Please review - and test:-)

ltp is a trainwreck.  Please send xfstests test like for all other file
system functionality.  You're really trying to make it as hard as
possible for fs developers, don't you?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add initial batch of statx() LTP tests
  2017-03-28  0:13         ` [PATCH] Add initial batch of statx() LTP tests David Howells
  2017-03-28  6:28           ` Christoph Hellwig
@ 2017-03-28  8:23           ` David Howells
  1 sibling, 0 replies; 38+ messages in thread
From: David Howells @ 2017-03-28  8:23 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: dhowells, Darrick J. Wong, Andreas Dilger, Eric Biggers,
	Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

Christoph Hellwig <hch@infradead.org> wrote:

> ltp is a trainwreck.  Please send xfstests test like for all other file
> system functionality.  You're really trying to make it as hard as
> possible for fs developers, don't you?

Sorry, Christoph, but, in my opinion, xfstests is in worse shape than LTP.
It's not self-contained (the code to make statx available seems to have to be
added to xfsprogs) and it's practically undocumented.  LTP, on the other hand,
is pretty easy to modify.

David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-27 19:04           ` Jeff Layton
@ 2017-03-28 19:39             ` Andreas Dilger
  2017-03-28 20:22               ` Jeff Layton
  2017-03-31 15:49             ` Christoph Hellwig
  1 sibling, 1 reply; 38+ messages in thread
From: Andreas Dilger @ 2017-03-28 19:39 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Christoph Hellwig, David Howells, Darrick J. Wong, Eric Biggers,
	Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

[-- Attachment #1: Type: text/plain, Size: 1635 bytes --]

On Mar 27, 2017, at 1:04 PM, Jeff Layton <jlayton@poochiereds.net> wrote:
> 
> On Mon, 2017-03-27 at 09:46 -0700, Christoph Hellwig wrote:
>> On Mon, Mar 27, 2017 at 05:25:26PM +0100, David Howells wrote:
>>> That means a filesystem can't simply return non-basic data unconditionally if
>>> possible.  I prefer letting it do so if it doesn't incur any extra I/O
>>> overheads.
>> 
>> This seems like it will lead to userspace expecting certain fields to
>> just be there, and a lot harder to properly verify for tests.  Which btw
>> we all need for these odd behaviors.  If we can't get them any time soon
>> (e.g. before -rc6) I think we'll simply have to revert statx instead of
>> leaving this untested mess in the tree.
> 
> I don't think so.
> 
> I think we just have to clearly document that that will not be the
> case. If they really expect the field to be there, then they need to
> set the bit in the "want" mask -- it's really as simple as that.

It would be my preference to only return attributes which are explicitly
requested (Lustre won't return fields for which the bit is not set), but
there was a request for this behaviour in NFS I recall.

That said, there are also flags for "lazy" attributes for NFS. I wonder if
it is enough to pass AT_STATX_DONT_SYNC and request STATX_SIZE | STATX_CTIME,
or whatever NFS is worried about, and return the client-local version of the
attributes?

I guess the real question is what an application is going to do with fields
that it didn't actually request, and has no way to know if they are valid or
contain garbage?

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-28 19:39             ` Andreas Dilger
@ 2017-03-28 20:22               ` Jeff Layton
  0 siblings, 0 replies; 38+ messages in thread
From: Jeff Layton @ 2017-03-28 20:22 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: Christoph Hellwig, David Howells, Darrick J. Wong, Eric Biggers,
	Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

On Tue, 2017-03-28 at 13:39 -0600, Andreas Dilger wrote:
> On Mar 27, 2017, at 1:04 PM, Jeff Layton <jlayton@poochiereds.net> wrote:
> > 
> > On Mon, 2017-03-27 at 09:46 -0700, Christoph Hellwig wrote:
> > > On Mon, Mar 27, 2017 at 05:25:26PM +0100, David Howells wrote:
> > > > That means a filesystem can't simply return non-basic data unconditionally if
> > > > possible.  I prefer letting it do so if it doesn't incur any extra I/O
> > > > overheads.
> > > 
> > > This seems like it will lead to userspace expecting certain fields to
> > > just be there, and a lot harder to properly verify for tests.  Which btw
> > > we all need for these odd behaviors.  If we can't get them any time soon
> > > (e.g. before -rc6) I think we'll simply have to revert statx instead of
> > > leaving this untested mess in the tree.
> > 
> > I don't think so.
> > 
> > I think we just have to clearly document that that will not be the
> > case. If they really expect the field to be there, then they need to
> > set the bit in the "want" mask -- it's really as simple as that.
> 
> It would be my preference to only return attributes which are explicitly
> requested (Lustre won't return fields for which the bit is not set), but
> there was a request for this behaviour in NFS I recall.
> 
> That said, there are also flags for "lazy" attributes for NFS. I wonder if
> it is enough to pass AT_STATX_DONT_SYNC and request STATX_SIZE | STATX_CTIME,
> or whatever NFS is worried about, and return the client-local version of the
> attributes?
> 
> I guess the real question is what an application is going to do with fields
> that it didn't actually request, and has no way to know if they are valid or
> contain garbage?
> 

You certainly do have a way to know if they're valid. You can check the
stx_mask.

Basically, if you request something, you're guaranteed to get it, or an
error. If you don't request it, you might get it but you had better
check the stx_mask before assuming that you did.

TBQH though, I don't _really_ expect userland to use this that often.

The main reason I'm arguing for this is for simplicity of the in-kernel 
implementations. Having to check each bit in the mask before filling
out its field is cumbersome. It's much simpler to fill out the kstat
struct with what you have and set the stx_mask unconditionally.

It's also not 100% clear to me how the unrequested fields would be set
if we don't allow this. zeroing them out seems like the obvious thing
to do, but that doesn't really provide any guarantee that someone won't
try to interpret it as valid.
-- 
Jeff Layton <jlayton@poochiereds.net>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-27 19:04           ` Jeff Layton
  2017-03-28 19:39             ` Andreas Dilger
@ 2017-03-31 15:49             ` Christoph Hellwig
  1 sibling, 0 replies; 38+ messages in thread
From: Christoph Hellwig @ 2017-03-31 15:49 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Christoph Hellwig, David Howells, Darrick J. Wong,
	Andreas Dilger, Eric Biggers, Michael Kerrisk (man-pages),
	linux-fsdevel, xfs

On Mon, Mar 27, 2017 at 03:04:11PM -0400, Jeff Layton wrote:
> I think we just have to clearly document that that will not be the
> case. If they really expect the field to be there, then they need to
> set the bit in the "want" mask -- it's really as simple as that.

If the usual local file systems set it but say NFS doesn't I will
very much expect that people don't set the flag and create buggy
programs a lot.  An API that doesn't give you more than you ask
for is a lot more obvious.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
                     ` (9 preceding siblings ...)
  2017-03-27  9:55   ` statx manpage David Howells
@ 2017-03-31 15:56   ` Christoph Hellwig
  2017-03-31 16:43   ` David Howells
  11 siblings, 0 replies; 38+ messages in thread
From: Christoph Hellwig @ 2017-03-31 15:56 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, mtk.manpages, Darrick J. Wong, linux-fsdevel,
	xfs, linux-man

On Tue, Mar 07, 2017 at 05:22:55PM +0000, David Howells wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > This would be great to have in 4.11 together with the initial statx
> > implementation.  But until I see documentation and testcases for statx
> > I don't really feel comfortable reviewing anything related to it.
> 
> Well, since you asked for documentation, here's a manual page for you to
> review:-)
> 
> Note that as it isn't in glibc yet, I've left out all the
> set-this-and-that-#define-to-make-it-appear stuff except where it is pertinent
> to particular constants.

I think we'll initially need to make this a section 2 page as libcs
will take a while to pick things up.  But in the end Michael will have
to decide.  Adding linux-man to the Cc list so that the right people
are involved.

Quoting the rendered page below:

>   Invoking statx():
 
 This is an odd header for a man page.  Usually this would start as

DESCRIPTION

(in bold).

>       [*] By absolute path.

Odd way to enumerate in a man page - I'll leave it to the linux-man regulars
to point out the right way, because I don't know it off-hand.

>       AT_EMPTY_PATH (since Linux 2.6.39)

All these have been there since statx was added, so we can remove the annotation.

>           STATX_BASIC_STATS   [The stuff in the normal stat struct]

Please enumerate the actual fields.

>           STATX_ALL           [All currently available stuff]

How about "All currently available fields"

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: statx manpage
  2017-03-07 17:22 ` statx manpage David Howells
                     ` (10 preceding siblings ...)
  2017-03-31 15:56   ` Christoph Hellwig
@ 2017-03-31 16:43   ` David Howells
  11 siblings, 0 replies; 38+ messages in thread
From: David Howells @ 2017-03-31 16:43 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: dhowells, mtk.manpages, Darrick J. Wong, linux-fsdevel, xfs, linux-man

Christoph Hellwig <hch@infradead.org> wrote:

> Quoting the rendered page below:
> 
> >   Invoking statx():
>  
>  This is an odd header for a man page.  Usually this would start as
> 
> DESCRIPTION
> 
> (in bold).

Yes, I know.  But it seems to make more sense this way round.  Putting the
struct definition before the description of how to access it means that I can
refer to it in the definitions of the flags that you OR into the mask and
means you get to the struct much faster.  Maybe I should put it under the
SYNOPSIS section?

> >       [*] By absolute path.
> 
> Odd way to enumerate in a man page - I'll leave it to the linux-man regulars
> to point out the right way, because I don't know it off-hand.

Me neither.

> >       AT_EMPTY_PATH (since Linux 2.6.39)
> 
> All these have been there since statx was added, so we can remove the
> annotation.

You're not the first to note this.  I've already taken that out.

> >           STATX_BASIC_STATS   [The stuff in the normal stat struct]
> 
> Please enumerate the actual fields.

They're enumerated immediately prior.  I've changed it to:

	STATX_BASIC_STATS   [All of the above]

> >           STATX_ALL           [All currently available stuff]
> 
> How about "All currently available fields"

Sure.

David

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2017-03-31 16:43 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-07  0:06 [PATCH] xfs: report crtime and attribute flags to statx Darrick J. Wong
2017-03-07  5:01 ` Christoph Hellwig
2017-03-07 17:23   ` Darrick J. Wong
2017-03-07 17:22 ` statx manpage David Howells
2017-03-07 18:02   ` Darrick J. Wong
2017-03-07 18:39   ` David Howells
2017-03-07 18:44     ` Darrick J. Wong
2017-03-07 18:55     ` David Howells
2017-03-07 21:44   ` Andreas Dilger
2017-03-07 22:55   ` Eric Biggers
2017-03-08  3:45   ` Eryu Guan
2017-03-08  9:24   ` David Howells
2017-03-08 15:26     ` Jeff Layton
2017-03-20 16:01       ` Matthew Wilcox
2017-03-22 10:55         ` Jeff Layton
2017-03-08  9:41   ` David Howells
2017-03-10  5:01     ` Eric Biggers
2017-03-09  6:46   ` David Howells
2017-03-09  6:59     ` Eryu Guan
2017-03-09  6:59     ` Darrick J. Wong
2017-03-09 14:01       ` Christoph Hellwig
2017-03-09  7:45     ` David Howells
2017-03-24 20:53   ` Eric Biggers
2017-03-27  0:46     ` Andreas Dilger
2017-03-27 15:40       ` Darrick J. Wong
2017-03-27 16:25       ` David Howells
2017-03-27 16:46         ` Christoph Hellwig
2017-03-27 19:04           ` Jeff Layton
2017-03-28 19:39             ` Andreas Dilger
2017-03-28 20:22               ` Jeff Layton
2017-03-31 15:49             ` Christoph Hellwig
2017-03-27 18:57         ` David Howells
2017-03-28  0:13         ` [PATCH] Add initial batch of statx() LTP tests David Howells
2017-03-28  6:28           ` Christoph Hellwig
2017-03-28  8:23           ` David Howells
2017-03-27  9:55   ` statx manpage David Howells
2017-03-31 15:56   ` Christoph Hellwig
2017-03-31 16:43   ` David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).