Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[David Howells]

Al Viro  wrote:

> > I understand wanting to avoid extra arguments, but you are asking for
> > trouble with that sort of calling conventions.  Verifying that all call
> > chains have these fields initialized is bloody unpleasant and it *is*
> > going to break, especially since the rules are "you need to initialize it
> > for vfs_xgetattr(), but not for vfs_getattr()" - the names are similar
> > enough for confusion, and that's not the only such pair.
> 
> BTW, after some grepping: you seem to leave ecryptfs_getattr() as-is, which
> means that flags it had been passed by caller in *stat are not seen by
> underlying fs.  Looks fishy...

I have patches for some other filesystems as well, but I'm trying to nail the
core down before whipping those back into shape.

David

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[David Howells]

Al Viro  wrote:

> > I understand wanting to avoid extra arguments, but you are asking for
> > trouble with that sort of calling conventions.  Verifying that all call
> > chains have these fields initialized is bloody unpleasant and it *is*
> > going to break, especially since the rules are "you need to initialize it
> > for vfs_xgetattr(), but not for vfs_getattr()" - the names are similar
> > enough for confusion, and that's not the only such pair.
> 
> BTW, after some grepping: you seem to leave ecryptfs_getattr() as-is, which
> means that flags it had been passed by caller in *stat are not seen by
> underlying fs.  Looks fishy...

I have patches for some other filesystems as well, but I'm trying to nail the
core down before whipping those back into shape.

David

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Miklos Szeredi]

On Sun, Dec 4, 2016 at 6:33 PM, Al Viro  wrote:
> On Sun, Dec 04, 2016 at 04:38:05AM +, Al Viro wrote:
>
>> I understand wanting to avoid extra arguments, but you are asking for trouble
>> with that sort of calling conventions.  Verifying that all call chains have
>> these fields initialized is bloody unpleasant and it *is* going to break,
>> especially since the rules are "you need to initialize it for vfs_xgetattr(),
>> but not for vfs_getattr()" - the names are similar enough for confusion,
>> and that's not the only such pair.
>
> FWIW, there's a bit of abuse of struct kstat in overlayfs object
> creation paths - for one thing, it ends up with a very small subset
> of struct kstat (mode + rdev), for another it also needs link in
> case of symlinks and ends up passing it separately.
>
> IMO it would be better to introduce a separate object for that; does anybody
> have objections to something like the patch below?  In principle, we might
> even lift that thing into general API and switch 
> ->mkdir()/->mknod()/->symlink()
> to identical calling conventions.  Hell knows, perhaps ->create() as well...
> Comments?

Good cleanup.  Applied, thanks.

Miklos

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Miklos Szeredi]

On Sun, Dec 4, 2016 at 6:33 PM, Al Viro  wrote:
> On Sun, Dec 04, 2016 at 04:38:05AM +, Al Viro wrote:
>
>> I understand wanting to avoid extra arguments, but you are asking for trouble
>> with that sort of calling conventions.  Verifying that all call chains have
>> these fields initialized is bloody unpleasant and it *is* going to break,
>> especially since the rules are "you need to initialize it for vfs_xgetattr(),
>> but not for vfs_getattr()" - the names are similar enough for confusion,
>> and that's not the only such pair.
>
> FWIW, there's a bit of abuse of struct kstat in overlayfs object
> creation paths - for one thing, it ends up with a very small subset
> of struct kstat (mode + rdev), for another it also needs link in
> case of symlinks and ends up passing it separately.
>
> IMO it would be better to introduce a separate object for that; does anybody
> have objections to something like the patch below?  In principle, we might
> even lift that thing into general API and switch 
> ->mkdir()/->mknod()/->symlink()
> to identical calling conventions.  Hell knows, perhaps ->create() as well...
> Comments?

Good cleanup.  Applied, thanks.

Miklos

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Al Viro]

On Sun, Dec 04, 2016 at 04:38:05AM +, Al Viro wrote:

> I understand wanting to avoid extra arguments, but you are asking for trouble
> with that sort of calling conventions.  Verifying that all call chains have
> these fields initialized is bloody unpleasant and it *is* going to break,
> especially since the rules are "you need to initialize it for vfs_xgetattr(),
> but not for vfs_getattr()" - the names are similar enough for confusion,
> and that's not the only such pair.

FWIW, there's a bit of abuse of struct kstat in overlayfs object
creation paths - for one thing, it ends up with a very small subset
of struct kstat (mode + rdev), for another it also needs link in
case of symlinks and ends up passing it separately.

IMO it would be better to introduce a separate object for that; does anybody
have objections to something like the patch below?  In principle, we might
even lift that thing into general API and switch ->mkdir()/->mknod()/->symlink()
to identical calling conventions.  Hell knows, perhaps ->create() as well...
Comments?

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 36795ee..17ad2da 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -231,10 +231,15 @@ static int ovl_copy_up_locked(struct dentry *workdir, 
struct dentry *upperdir,
struct inode *udir = upperdir->d_inode;
struct dentry *newdentry = NULL;
struct dentry *upper = NULL;
-   umode_t mode = stat->mode;
int err;
const struct cred *old_creds = NULL;
struct cred *new_creds = NULL;
+   struct cattr cattr = {
+   /* Can't properly set mode on creation because of the umask */
+   .mode = stat->mode & S_IFMT,
+   .rdev = stat->rdev,
+   .link = link
+   };
 
newdentry = ovl_lookup_temp(workdir, dentry);
err = PTR_ERR(newdentry);
@@ -254,10 +259,7 @@ static int ovl_copy_up_locked(struct dentry *workdir, 
struct dentry *upperdir,
if (new_creds)
old_creds = override_creds(new_creds);
 
-   /* Can't properly set mode on creation because of the umask */
-   stat->mode &= S_IFMT;
-   err = ovl_create_real(wdir, newdentry, stat, link, NULL, true);
-   stat->mode = mode;
+   err = ovl_create_real(wdir, newdentry, , NULL, true);
 
if (new_creds) {
revert_creds(old_creds);
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 306b6c1..dc60a23 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -75,8 +75,8 @@ static struct dentry *ovl_whiteout(struct dentry *workdir,
 }
 
 int ovl_create_real(struct inode *dir, struct dentry *newdentry,
-   struct kstat *stat, const char *link,
-   struct dentry *hardlink, bool debug)
+   struct cattr *attr, struct dentry *hardlink,
+   bool debug)
 {
int err;
 
@@ -86,13 +86,13 @@ int ovl_create_real(struct inode *dir, struct dentry 
*newdentry,
if (hardlink) {
err = ovl_do_link(hardlink, dir, newdentry, debug);
} else {
-   switch (stat->mode & S_IFMT) {
+   switch (attr->mode & S_IFMT) {
case S_IFREG:
-   err = ovl_do_create(dir, newdentry, stat->mode, debug);
+   err = ovl_do_create(dir, newdentry, attr->mode, debug);
break;
 
case S_IFDIR:
-   err = ovl_do_mkdir(dir, newdentry, stat->mode, debug);
+   err = ovl_do_mkdir(dir, newdentry, attr->mode, debug);
break;
 
case S_IFCHR:
@@ -100,11 +100,11 @@ int ovl_create_real(struct inode *dir, struct dentry 
*newdentry,
case S_IFIFO:
case S_IFSOCK:
err = ovl_do_mknod(dir, newdentry,
-  stat->mode, stat->rdev, debug);
+  attr->mode, attr->rdev, debug);
break;
 
case S_IFLNK:
-   err = ovl_do_symlink(dir, newdentry, link, debug);
+   err = ovl_do_symlink(dir, newdentry, attr->link, debug);
break;
 
default:
@@ -183,8 +183,7 @@ static void ovl_instantiate(struct dentry *dentry, struct 
inode *inode,
 }
 
 static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
-   struct kstat *stat, const char *link,
-   struct dentry *hardlink)
+   struct cattr *attr, struct dentry *hardlink)
 {
struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
struct inode *udir = upperdir->d_inode;
@@ -192,7 +191,7 @@ static int ovl_create_upper(struct dentry *dentry, struct 
inode *inode,
int err;
 
if (!hardlink && !IS_POSIXACL(udir))
-   stat->mode &= 

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Al Viro]

On Sun, Dec 04, 2016 at 04:38:05AM +, Al Viro wrote:

> I understand wanting to avoid extra arguments, but you are asking for trouble
> with that sort of calling conventions.  Verifying that all call chains have
> these fields initialized is bloody unpleasant and it *is* going to break,
> especially since the rules are "you need to initialize it for vfs_xgetattr(),
> but not for vfs_getattr()" - the names are similar enough for confusion,
> and that's not the only such pair.

FWIW, there's a bit of abuse of struct kstat in overlayfs object
creation paths - for one thing, it ends up with a very small subset
of struct kstat (mode + rdev), for another it also needs link in
case of symlinks and ends up passing it separately.

IMO it would be better to introduce a separate object for that; does anybody
have objections to something like the patch below?  In principle, we might
even lift that thing into general API and switch ->mkdir()/->mknod()/->symlink()
to identical calling conventions.  Hell knows, perhaps ->create() as well...
Comments?

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 36795ee..17ad2da 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -231,10 +231,15 @@ static int ovl_copy_up_locked(struct dentry *workdir, 
struct dentry *upperdir,
struct inode *udir = upperdir->d_inode;
struct dentry *newdentry = NULL;
struct dentry *upper = NULL;
-   umode_t mode = stat->mode;
int err;
const struct cred *old_creds = NULL;
struct cred *new_creds = NULL;
+   struct cattr cattr = {
+   /* Can't properly set mode on creation because of the umask */
+   .mode = stat->mode & S_IFMT,
+   .rdev = stat->rdev,
+   .link = link
+   };
 
newdentry = ovl_lookup_temp(workdir, dentry);
err = PTR_ERR(newdentry);
@@ -254,10 +259,7 @@ static int ovl_copy_up_locked(struct dentry *workdir, 
struct dentry *upperdir,
if (new_creds)
old_creds = override_creds(new_creds);
 
-   /* Can't properly set mode on creation because of the umask */
-   stat->mode &= S_IFMT;
-   err = ovl_create_real(wdir, newdentry, stat, link, NULL, true);
-   stat->mode = mode;
+   err = ovl_create_real(wdir, newdentry, , NULL, true);
 
if (new_creds) {
revert_creds(old_creds);
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 306b6c1..dc60a23 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -75,8 +75,8 @@ static struct dentry *ovl_whiteout(struct dentry *workdir,
 }
 
 int ovl_create_real(struct inode *dir, struct dentry *newdentry,
-   struct kstat *stat, const char *link,
-   struct dentry *hardlink, bool debug)
+   struct cattr *attr, struct dentry *hardlink,
+   bool debug)
 {
int err;
 
@@ -86,13 +86,13 @@ int ovl_create_real(struct inode *dir, struct dentry 
*newdentry,
if (hardlink) {
err = ovl_do_link(hardlink, dir, newdentry, debug);
} else {
-   switch (stat->mode & S_IFMT) {
+   switch (attr->mode & S_IFMT) {
case S_IFREG:
-   err = ovl_do_create(dir, newdentry, stat->mode, debug);
+   err = ovl_do_create(dir, newdentry, attr->mode, debug);
break;
 
case S_IFDIR:
-   err = ovl_do_mkdir(dir, newdentry, stat->mode, debug);
+   err = ovl_do_mkdir(dir, newdentry, attr->mode, debug);
break;
 
case S_IFCHR:
@@ -100,11 +100,11 @@ int ovl_create_real(struct inode *dir, struct dentry 
*newdentry,
case S_IFIFO:
case S_IFSOCK:
err = ovl_do_mknod(dir, newdentry,
-  stat->mode, stat->rdev, debug);
+  attr->mode, attr->rdev, debug);
break;
 
case S_IFLNK:
-   err = ovl_do_symlink(dir, newdentry, link, debug);
+   err = ovl_do_symlink(dir, newdentry, attr->link, debug);
break;
 
default:
@@ -183,8 +183,7 @@ static void ovl_instantiate(struct dentry *dentry, struct 
inode *inode,
 }
 
 static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
-   struct kstat *stat, const char *link,
-   struct dentry *hardlink)
+   struct cattr *attr, struct dentry *hardlink)
 {
struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
struct inode *udir = upperdir->d_inode;
@@ -192,7 +191,7 @@ static int ovl_create_upper(struct dentry *dentry, struct 
inode *inode,
int err;
 
if (!hardlink && !IS_POSIXACL(udir))
-   stat->mode &= 

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Al Viro]

On Sun, Dec 04, 2016 at 04:38:05AM +, Al Viro wrote:

> I understand wanting to avoid extra arguments, but you are asking for trouble
> with that sort of calling conventions.  Verifying that all call chains have
> these fields initialized is bloody unpleasant and it *is* going to break,
> especially since the rules are "you need to initialize it for vfs_xgetattr(),
> but not for vfs_getattr()" - the names are similar enough for confusion,
> and that's not the only such pair.

BTW, after some grepping: you seem to leave ecryptfs_getattr() as-is, which
means that flags it had been passed by caller in *stat are not seen by
underlying fs.  Looks fishy...

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Al Viro]

On Sun, Dec 04, 2016 at 04:38:05AM +, Al Viro wrote:

> I understand wanting to avoid extra arguments, but you are asking for trouble
> with that sort of calling conventions.  Verifying that all call chains have
> these fields initialized is bloody unpleasant and it *is* going to break,
> especially since the rules are "you need to initialize it for vfs_xgetattr(),
> but not for vfs_getattr()" - the names are similar enough for confusion,
> and that's not the only such pair.

BTW, after some grepping: you seem to leave ecryptfs_getattr() as-is, which
means that flags it had been passed by caller in *stat are not seen by
underlying fs.  Looks fishy...

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Al Viro]

On Wed, Nov 23, 2016 at 12:55:51AM +, David Howells wrote:

> + * vfs_xgetattr - Get the enhanced basic attributes of a file
> + * @path: The file of interest
> + * @stat: Where to return the statistics
> + *
> + * Ask the filesystem for a file's attributes.  The caller must have preset
> + * stat->request_mask and stat->query_flags to indicate what they want.
> + *
> + * If the file is remote, the filesystem can be forced to update the 
> attributes
> + * from the backing store by passing AT_FORCE_ATTR_SYNC in query_flags or can
> + * suppress the update by passing AT_NO_ATTR_SYNC.
> + *
> + * Bits must have been set in stat->request_mask to indicate which attributes
> + * the caller wants retrieving.  Any such attribute not requested may be
> + * returned anyway, but the value may be approximate, and, if remote, may not
> + * have been synchronised with the server.

I understand wanting to avoid extra arguments, but you are asking for trouble
with that sort of calling conventions.  Verifying that all call chains have
these fields initialized is bloody unpleasant and it *is* going to break,
especially since the rules are "you need to initialize it for vfs_xgetattr(),
but not for vfs_getattr()" - the names are similar enough for confusion,
and that's not the only such pair.

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Al Viro]

On Wed, Nov 23, 2016 at 12:55:51AM +, David Howells wrote:

> + * vfs_xgetattr - Get the enhanced basic attributes of a file
> + * @path: The file of interest
> + * @stat: Where to return the statistics
> + *
> + * Ask the filesystem for a file's attributes.  The caller must have preset
> + * stat->request_mask and stat->query_flags to indicate what they want.
> + *
> + * If the file is remote, the filesystem can be forced to update the 
> attributes
> + * from the backing store by passing AT_FORCE_ATTR_SYNC in query_flags or can
> + * suppress the update by passing AT_NO_ATTR_SYNC.
> + *
> + * Bits must have been set in stat->request_mask to indicate which attributes
> + * the caller wants retrieving.  Any such attribute not requested may be
> + * returned anyway, but the value may be approximate, and, if remote, may not
> + * have been synchronised with the server.

I understand wanting to avoid extra arguments, but you are asking for trouble
with that sort of calling conventions.  Verifying that all call chains have
these fields initialized is bloody unpleasant and it *is* going to break,
especially since the rules are "you need to initialize it for vfs_xgetattr(),
but not for vfs_getattr()" - the names are similar enough for confusion,
and that's not the only such pair.

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Al Viro]

On Wed, Nov 23, 2016 at 12:55:51AM +, David Howells wrote:

> -int vfs_getattr_nosec(struct path *path, struct kstat *stat)
> +int vfs_xgetattr_nosec(struct path *path, struct kstat *stat)

const struct path *, please.  Especially since we feed >f_path to
it.  And yes, I realize that the same applies to vfs_getattr() in the
current tree.

> +int vfs_xgetattr(struct path *path, struct kstat *stat)

Ditto.

> +int vfs_getattr(struct path *path, struct kstat *stat)

Again.

> +#define __put_timestamp(kts, uts) (  \
> + __put_user(kts.tv_sec,  uts.tv_sec  ) ||\
> + __put_user(kts.tv_nsec, uts.tv_nsec ) ||\
> + __put_user(0,   uts.__reserved  ))

static inline.

> +#define KSTAT_ATTR_FS_IOC_FLAGS  0x0874 /* Attrs 
> corresponding to FS_*_FL flags */

Err...  An explicit hex constant?

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Al Viro]

On Wed, Nov 23, 2016 at 12:55:51AM +, David Howells wrote:

> -int vfs_getattr_nosec(struct path *path, struct kstat *stat)
> +int vfs_xgetattr_nosec(struct path *path, struct kstat *stat)

const struct path *, please.  Especially since we feed >f_path to
it.  And yes, I realize that the same applies to vfs_getattr() in the
current tree.

> +int vfs_xgetattr(struct path *path, struct kstat *stat)

Ditto.

> +int vfs_getattr(struct path *path, struct kstat *stat)

Again.

> +#define __put_timestamp(kts, uts) (  \
> + __put_user(kts.tv_sec,  uts.tv_sec  ) ||\
> + __put_user(kts.tv_nsec, uts.tv_nsec ) ||\
> + __put_user(0,   uts.__reserved  ))

static inline.

> +#define KSTAT_ATTR_FS_IOC_FLAGS  0x0874 /* Attrs 
> corresponding to FS_*_FL flags */

Err...  An explicit hex constant?

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Michael Kerrisk (man-pages)]

Hi Andreas,

On 11/23/2016 11:57 PM, Andreas Dilger wrote:
> On Nov 23, 2016, at 1:37 AM, Michael Kerrisk  wrote:
>>

[...]

>>> ===
>>> NEW SYSTEM CALL
>>> ===
>>>
>>> The new system call is:
>>>
>>> int ret = statx(int dfd,
>>> const char *filename,
>>> unsigned int flags,
>>
>> In the 0/4 of this patch series, this argument is called 'atflags'.
>> These should be consistent. 'flags' seems correct to me.
> 
> Given that there are a number of different flags and masks in use for
> this syscall, naming this field "atflags" makes it more clear what it
> is used for.

Well, yes, but in all of the "at" calls the argument is just flags.
So, better to be consistent in that dimension, I'd say, and then 
make sure we perhaps have meaningfil names for the fiellds, as you suggest next:


>>> unsigned int mask,
> 
> Similarly, naming this field "request_mask" would also be more clear,
> and matches what is used elsewhere in the code.
> 
> That said, I don't care enough about this detail to request a patch refresh,
> but it would be useful for the man pages.


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Michael Kerrisk (man-pages)]

Hi Andreas,

On 11/23/2016 11:57 PM, Andreas Dilger wrote:
> On Nov 23, 2016, at 1:37 AM, Michael Kerrisk  wrote:
>>

[...]

>>> ===
>>> NEW SYSTEM CALL
>>> ===
>>>
>>> The new system call is:
>>>
>>> int ret = statx(int dfd,
>>> const char *filename,
>>> unsigned int flags,
>>
>> In the 0/4 of this patch series, this argument is called 'atflags'.
>> These should be consistent. 'flags' seems correct to me.
> 
> Given that there are a number of different flags and masks in use for
> this syscall, naming this field "atflags" makes it more clear what it
> is used for.

Well, yes, but in all of the "at" calls the argument is just flags.
So, better to be consistent in that dimension, I'd say, and then 
make sure we perhaps have meaningfil names for the fiellds, as you suggest next:


>>> unsigned int mask,
> 
> Similarly, naming this field "request_mask" would also be more clear,
> and matches what is used elsewhere in the code.
> 
> That said, I don't care enough about this detail to request a patch refresh,
> but it would be useful for the man pages.


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[David Howells]

Andreas Dilger  wrote:

> > +   case S_IFCHR:   printf(" character special file\n");ft = 
> > 'c'; break;
> 
> This will overflow 80 columns.  Could use just "character special"?
> 
> > +   case S_IFDIR:   printf(" directory\n"); ft = 
> > 'd'; break;
> > +   case S_IFBLK:   printf(" block special file\n");ft = 
> > 'b'; break;
> > +   case S_IFREG:   printf(" regular file\n");  ft = 
> > '-'; break;
> > +   case S_IFLNK:   printf(" symbolic link\n"); ft = 
> > 'l'; break;
> 
> Having these all as single-word items would make it easier to parse for
> test programs.  Same with "IO Block:" -> "Blocksize:".  Not a huge deal.

Try doing "stat /dev/null".

David

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[David Howells]

Andreas Dilger  wrote:

> > +   case S_IFCHR:   printf(" character special file\n");ft = 
> > 'c'; break;
> 
> This will overflow 80 columns.  Could use just "character special"?
> 
> > +   case S_IFDIR:   printf(" directory\n"); ft = 
> > 'd'; break;
> > +   case S_IFBLK:   printf(" block special file\n");ft = 
> > 'b'; break;
> > +   case S_IFREG:   printf(" regular file\n");  ft = 
> > '-'; break;
> > +   case S_IFLNK:   printf(" symbolic link\n"); ft = 
> > 'l'; break;
> 
> Having these all as single-word items would make it easier to parse for
> test programs.  Same with "IO Block:" -> "Blocksize:".  Not a huge deal.

Try doing "stat /dev/null".

David

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Andreas Dilger]

> On Nov 22, 2016, at 5:55 PM, David Howells  wrote:
> 
> Add a system call to make extended file information available, including
> file creation and some attribute flags where available through the
> underlying filesystem.
> 
> 
> 
> OVERVIEW
> 
> 
> The idea was initially proposed as a set of xattrs that could be retrieved
> with getxattr(), but the general preferance proved to be for a new syscall
> with an extended stat structure.
> 
> This can feasibly be used to support a number of things, not all of which
> are added here:
> 
> (1) Better support for the y2038 problem [Arnd Bergmann].
> 
> (2) Creation time: The SMB protocol carries the creation time, which could
> be exported by Samba, which will in turn help CIFS make use of
> FS-Cache as that can be used for coherency data.
> 
> This is also specified in NFSv4 as a recommended attribute and could
> be exported by NFSD [Steve French].
> 
> (3) Lightweight stat: Ask for just those details of interest, and allow a
> netfs (such as NFS) to approximate anything not of interest, possibly
> without going to the server [Trond Myklebust, Ulrich Drepper, Andreas
> Dilger].
> 
> (4) Heavyweight stat: Force a netfs to go to the server, even if it thinks
> its cached attributes are up to date [Trond Myklebust].
> 
> (5) Data version number: Could be used by userspace NFS servers [Aneesh
> Kumar].
> 
> Can also be used to modify fill_post_wcc() in NFSD which retrieves
> i_version directly, but has just called vfs_getattr().  It could get
> it from the kstat struct if it used vfs_xgetattr() instead.
> 
> (6) BSD stat compatibility: Including more fields from the BSD stat such
> as creation time (st_btime) and inode generation number (st_gen)
> [Jeremy Allison, Bernd Schubert].
> 
> (7) Inode generation number: Useful for FUSE and userspace NFS servers
> [Bernd Schubert].  This was asked for but later deemed unnecessary
> with the open-by-handle capability available
> 
> (8) Extra coherency data may be useful in making backups [Andreas Dilger].
> 
> (9) Allow the filesystem to indicate what it can/cannot provide: A
> filesystem can now say it doesn't support a standard stat feature if
> that isn't available, so if, for instance, inode numbers or UIDs don't
> exist or are fabricated locally...
> 
> (10) Make the fields a consistent size on all arches and make them large.
> 
> (11) Store a 16-byte volume ID in the superblock that can be returned in
> struct xstat [Steve French].
> 
> (12) Include granularity fields in the time data to indicate the
> granularity of each of the times (NFSv4 time_delta) [Steve French].
> 
> (13) FS_IOC_GETFLAGS value.  These could be translated to BSD's st_flags.
> Note that the Linux IOC flags are a mess and filesystems such as Ext4
> define flags that aren't in linux/fs.h, so translation in the kernel
> may be a necessity (or, possibly, we provide the filesystem type too).
> 
> (14) Mask of features available on file (eg: ACLs, seclabel) [Brad Boyer,
> Michael Kerrisk].
> 
> (15) Spare space, request flags and information flags are provided for
> future expansion.
> 
> (16) Femtosecond-resolution timestamps [Dave Chinner].
> 
> 
> ===
> NEW SYSTEM CALL
> ===
> 
> The new system call is:
> 
>   int ret = statx(int dfd,
>   const char *filename,
>   unsigned int flags,
>   unsigned int mask,
>   struct statx *buffer);
> 
> The dfd, filename and flags parameters indicate the file to query, in a
> similar way to fstatat().  There is no equivalent of lstat() as that can be
> emulated with statx() by passing AT_SYMLINK_NOFOLLOW in flags.  There is
> also no equivalent of fstat() as that can be emulated by passing a NULL
> filename to statx() with the fd of interest in dfd.
> 
> Whether or not statx() synchronises the attributes with the backing store
> can be controlled (this typically only affects network filesystems) can be
> set by OR'ing a value into the flags argument:
> 
> (1) AT_STATX_SYNC_AS_STAT tells statx() to behave as stat() does in this
> respect.
> 
> (2) AT_STATX_FORCE_SYNC will require a network filesystem to synchronise
> its attributes with the server - which might require data writeback to
> occur to get the timestamps correct.
> 
> (3) AT_STATX_DONT_SYNC will suppress synchronisation with the server in a
> network filesystem.  The resulting values should be considered
> approximate.
> 
> mask is a bitmask indicating the fields in struct statx that are of
> interest to the caller.  The user should set this to STATX_BASIC_STATS to
> get the basic set returned by stat().  It should be note that asking for
> more information may entail extra I/O operations.
> 
> buffer points to the destination for the data.  This must be 256 bytes in
> size.
> 
> 
> 

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Andreas Dilger]

> On Nov 22, 2016, at 5:55 PM, David Howells  wrote:
> 
> Add a system call to make extended file information available, including
> file creation and some attribute flags where available through the
> underlying filesystem.
> 
> 
> 
> OVERVIEW
> 
> 
> The idea was initially proposed as a set of xattrs that could be retrieved
> with getxattr(), but the general preferance proved to be for a new syscall
> with an extended stat structure.
> 
> This can feasibly be used to support a number of things, not all of which
> are added here:
> 
> (1) Better support for the y2038 problem [Arnd Bergmann].
> 
> (2) Creation time: The SMB protocol carries the creation time, which could
> be exported by Samba, which will in turn help CIFS make use of
> FS-Cache as that can be used for coherency data.
> 
> This is also specified in NFSv4 as a recommended attribute and could
> be exported by NFSD [Steve French].
> 
> (3) Lightweight stat: Ask for just those details of interest, and allow a
> netfs (such as NFS) to approximate anything not of interest, possibly
> without going to the server [Trond Myklebust, Ulrich Drepper, Andreas
> Dilger].
> 
> (4) Heavyweight stat: Force a netfs to go to the server, even if it thinks
> its cached attributes are up to date [Trond Myklebust].
> 
> (5) Data version number: Could be used by userspace NFS servers [Aneesh
> Kumar].
> 
> Can also be used to modify fill_post_wcc() in NFSD which retrieves
> i_version directly, but has just called vfs_getattr().  It could get
> it from the kstat struct if it used vfs_xgetattr() instead.
> 
> (6) BSD stat compatibility: Including more fields from the BSD stat such
> as creation time (st_btime) and inode generation number (st_gen)
> [Jeremy Allison, Bernd Schubert].
> 
> (7) Inode generation number: Useful for FUSE and userspace NFS servers
> [Bernd Schubert].  This was asked for but later deemed unnecessary
> with the open-by-handle capability available
> 
> (8) Extra coherency data may be useful in making backups [Andreas Dilger].
> 
> (9) Allow the filesystem to indicate what it can/cannot provide: A
> filesystem can now say it doesn't support a standard stat feature if
> that isn't available, so if, for instance, inode numbers or UIDs don't
> exist or are fabricated locally...
> 
> (10) Make the fields a consistent size on all arches and make them large.
> 
> (11) Store a 16-byte volume ID in the superblock that can be returned in
> struct xstat [Steve French].
> 
> (12) Include granularity fields in the time data to indicate the
> granularity of each of the times (NFSv4 time_delta) [Steve French].
> 
> (13) FS_IOC_GETFLAGS value.  These could be translated to BSD's st_flags.
> Note that the Linux IOC flags are a mess and filesystems such as Ext4
> define flags that aren't in linux/fs.h, so translation in the kernel
> may be a necessity (or, possibly, we provide the filesystem type too).
> 
> (14) Mask of features available on file (eg: ACLs, seclabel) [Brad Boyer,
> Michael Kerrisk].
> 
> (15) Spare space, request flags and information flags are provided for
> future expansion.
> 
> (16) Femtosecond-resolution timestamps [Dave Chinner].
> 
> 
> ===
> NEW SYSTEM CALL
> ===
> 
> The new system call is:
> 
>   int ret = statx(int dfd,
>   const char *filename,
>   unsigned int flags,
>   unsigned int mask,
>   struct statx *buffer);
> 
> The dfd, filename and flags parameters indicate the file to query, in a
> similar way to fstatat().  There is no equivalent of lstat() as that can be
> emulated with statx() by passing AT_SYMLINK_NOFOLLOW in flags.  There is
> also no equivalent of fstat() as that can be emulated by passing a NULL
> filename to statx() with the fd of interest in dfd.
> 
> Whether or not statx() synchronises the attributes with the backing store
> can be controlled (this typically only affects network filesystems) can be
> set by OR'ing a value into the flags argument:
> 
> (1) AT_STATX_SYNC_AS_STAT tells statx() to behave as stat() does in this
> respect.
> 
> (2) AT_STATX_FORCE_SYNC will require a network filesystem to synchronise
> its attributes with the server - which might require data writeback to
> occur to get the timestamps correct.
> 
> (3) AT_STATX_DONT_SYNC will suppress synchronisation with the server in a
> network filesystem.  The resulting values should be considered
> approximate.
> 
> mask is a bitmask indicating the fields in struct statx that are of
> interest to the caller.  The user should set this to STATX_BASIC_STATS to
> get the basic set returned by stat().  It should be note that asking for
> more information may entail extra I/O operations.
> 
> buffer points to the destination for the data.  This must be 256 bytes in
> size.
> 
> 
> ==
> 

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Andreas Dilger]

On Nov 23, 2016, at 1:37 AM, Michael Kerrisk  wrote:
> 
> Hi David,
> 
> On 11/23/2016 01:55 AM, David Howells wrote:
>> Add a system call to make extended file information available, including
>> file creation and some attribute flags where available through the
>> underlying filesystem.
>> 
>> 
>> 
>> OVERVIEW
>> 
>> 
>> The idea was initially proposed as a set of xattrs that could be retrieved
>> with getxattr(), but the general preferance proved to be for a new syscall
> 
> s/preferance/preference/
> 
>> with an extended stat structure.
>> 
>> This can feasibly be used to support a number of things, not all of which
>> are added here:
> 
> It would be very useful if this overview distinguishes which of the features
> below are supported in the initial implementation, versus which features
> (e.g., femtosecond timestamps) are simply allowed for in a future
> implementation.
> 
>> (1) Better support for the y2038 problem [Arnd Bergmann].
>> 
>> (2) Creation time: The SMB protocol carries the creation time, which could
>> be exported by Samba, which will in turn help CIFS make use of
>> FS-Cache as that can be used for coherency data.
>> 
>> This is also specified in NFSv4 as a recommended attribute and could
>> be exported by NFSD [Steve French].
>> 
>> (3) Lightweight stat: Ask for just those details of interest, and allow a
>> netfs (such as NFS) to approximate anything not of interest, possibly
>> without going to the server [Trond Myklebust, Ulrich Drepper, Andreas
>> Dilger].
>> 
>> (4) Heavyweight stat: Force a netfs to go to the server, even if it thinks
>> its cached attributes are up to date [Trond Myklebust].
>> 
>> (5) Data version number: Could be used by userspace NFS servers [Aneesh
>> Kumar].
>> 
>> Can also be used to modify fill_post_wcc() in NFSD which retrieves
>> i_version directly, but has just called vfs_getattr().  It could get
>> it from the kstat struct if it used vfs_xgetattr() instead.
>> 
>> (6) BSD stat compatibility: Including more fields from the BSD stat such
>> as creation time (st_btime) and inode generation number (st_gen)
>> [Jeremy Allison, Bernd Schubert].
>> 
>> (7) Inode generation number: Useful for FUSE and userspace NFS servers
>> [Bernd Schubert].  This was asked for but later deemed unnecessary
>> with the open-by-handle capability available
>> 
>> (8) Extra coherency data may be useful in making backups [Andreas Dilger].
> 
> Can you elaborate on the point [8] in this commit message. It's not clear
> to me at least what this is about.
>> 
>> (9) Allow the filesystem to indicate what it can/cannot provide: A
>> filesystem can now say it doesn't support a standard stat feature if
>> that isn't available, so if, for instance, inode numbers or UIDs don't
>> exist or are fabricated locally...
>> 
>> (10) Make the fields a consistent size on all arches and make them large.
>> 
>> (11) Store a 16-byte volume ID in the superblock that can be returned in
>> struct xstat [Steve French].
>> 
>> (12) Include granularity fields in the time data to indicate the
>> granularity of each of the times (NFSv4 time_delta) [Steve French].
>> 
>> (13) FS_IOC_GETFLAGS value.  These could be translated to BSD's st_flags.
>> Note that the Linux IOC flags are a mess and filesystems such as Ext4
>> define flags that aren't in linux/fs.h, so translation in the kernel
>> may be a necessity (or, possibly, we provide the filesystem type too).
>> 
>> (14) Mask of features available on file (eg: ACLs, seclabel) [Brad Boyer,
>> Michael Kerrisk].
>> 
>> (15) Spare space, request flags and information flags are provided for
>> future expansion.
>> 
>> (16) Femtosecond-resolution timestamps [Dave Chinner].
>> 
>> 
>> ===
>> NEW SYSTEM CALL
>> ===
>> 
>> The new system call is:
>> 
>>  int ret = statx(int dfd,
>>  const char *filename,
>>  unsigned int flags,
> 
> In the 0/4 of this patch series, this argument is called 'atflags'.
> These should be consistent. 'flags' seems correct to me.

Given that there are a number of different flags and masks in use for
this syscall, naming this field "atflags" makes it more clear what it
is used for.

>>  unsigned int mask,

Similarly, naming this field "request_mask" would also be more clear,
and matches what is used elsewhere in the code.

That said, I don't care enough about this detail to request a patch refresh,
but it would be useful for the man pages.

Cheers, Andreas

>>  struct statx *buffer);
>> 
>> The dfd, filename and flags parameters indicate the file to query, in a
>> similar way to fstatat().  There is no equivalent of lstat() as that can be
>> emulated with statx() by passing AT_SYMLINK_NOFOLLOW in flags.  There is
>> also no equivalent of fstat() as that can be emulated by passing a NULL
>> 

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Andreas Dilger]

On Nov 23, 2016, at 1:37 AM, Michael Kerrisk  wrote:
> 
> Hi David,
> 
> On 11/23/2016 01:55 AM, David Howells wrote:
>> Add a system call to make extended file information available, including
>> file creation and some attribute flags where available through the
>> underlying filesystem.
>> 
>> 
>> 
>> OVERVIEW
>> 
>> 
>> The idea was initially proposed as a set of xattrs that could be retrieved
>> with getxattr(), but the general preferance proved to be for a new syscall
> 
> s/preferance/preference/
> 
>> with an extended stat structure.
>> 
>> This can feasibly be used to support a number of things, not all of which
>> are added here:
> 
> It would be very useful if this overview distinguishes which of the features
> below are supported in the initial implementation, versus which features
> (e.g., femtosecond timestamps) are simply allowed for in a future
> implementation.
> 
>> (1) Better support for the y2038 problem [Arnd Bergmann].
>> 
>> (2) Creation time: The SMB protocol carries the creation time, which could
>> be exported by Samba, which will in turn help CIFS make use of
>> FS-Cache as that can be used for coherency data.
>> 
>> This is also specified in NFSv4 as a recommended attribute and could
>> be exported by NFSD [Steve French].
>> 
>> (3) Lightweight stat: Ask for just those details of interest, and allow a
>> netfs (such as NFS) to approximate anything not of interest, possibly
>> without going to the server [Trond Myklebust, Ulrich Drepper, Andreas
>> Dilger].
>> 
>> (4) Heavyweight stat: Force a netfs to go to the server, even if it thinks
>> its cached attributes are up to date [Trond Myklebust].
>> 
>> (5) Data version number: Could be used by userspace NFS servers [Aneesh
>> Kumar].
>> 
>> Can also be used to modify fill_post_wcc() in NFSD which retrieves
>> i_version directly, but has just called vfs_getattr().  It could get
>> it from the kstat struct if it used vfs_xgetattr() instead.
>> 
>> (6) BSD stat compatibility: Including more fields from the BSD stat such
>> as creation time (st_btime) and inode generation number (st_gen)
>> [Jeremy Allison, Bernd Schubert].
>> 
>> (7) Inode generation number: Useful for FUSE and userspace NFS servers
>> [Bernd Schubert].  This was asked for but later deemed unnecessary
>> with the open-by-handle capability available
>> 
>> (8) Extra coherency data may be useful in making backups [Andreas Dilger].
> 
> Can you elaborate on the point [8] in this commit message. It's not clear
> to me at least what this is about.
>> 
>> (9) Allow the filesystem to indicate what it can/cannot provide: A
>> filesystem can now say it doesn't support a standard stat feature if
>> that isn't available, so if, for instance, inode numbers or UIDs don't
>> exist or are fabricated locally...
>> 
>> (10) Make the fields a consistent size on all arches and make them large.
>> 
>> (11) Store a 16-byte volume ID in the superblock that can be returned in
>> struct xstat [Steve French].
>> 
>> (12) Include granularity fields in the time data to indicate the
>> granularity of each of the times (NFSv4 time_delta) [Steve French].
>> 
>> (13) FS_IOC_GETFLAGS value.  These could be translated to BSD's st_flags.
>> Note that the Linux IOC flags are a mess and filesystems such as Ext4
>> define flags that aren't in linux/fs.h, so translation in the kernel
>> may be a necessity (or, possibly, we provide the filesystem type too).
>> 
>> (14) Mask of features available on file (eg: ACLs, seclabel) [Brad Boyer,
>> Michael Kerrisk].
>> 
>> (15) Spare space, request flags and information flags are provided for
>> future expansion.
>> 
>> (16) Femtosecond-resolution timestamps [Dave Chinner].
>> 
>> 
>> ===
>> NEW SYSTEM CALL
>> ===
>> 
>> The new system call is:
>> 
>>  int ret = statx(int dfd,
>>  const char *filename,
>>  unsigned int flags,
> 
> In the 0/4 of this patch series, this argument is called 'atflags'.
> These should be consistent. 'flags' seems correct to me.

Given that there are a number of different flags and masks in use for
this syscall, naming this field "atflags" makes it more clear what it
is used for.

>>  unsigned int mask,

Similarly, naming this field "request_mask" would also be more clear,
and matches what is used elsewhere in the code.

That said, I don't care enough about this detail to request a patch refresh,
but it would be useful for the man pages.

Cheers, Andreas

>>  struct statx *buffer);
>> 
>> The dfd, filename and flags parameters indicate the file to query, in a
>> similar way to fstatat().  There is no equivalent of lstat() as that can be
>> emulated with statx() by passing AT_SYMLINK_NOFOLLOW in flags.  There is
>> also no equivalent of fstat() as that can be emulated by passing a NULL
>> filename to statx() with 

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Andreas Dilger]

On Nov 23, 2016, at 1:37 AM, Michael Kerrisk  wrote:
> 
> Hi David,
> 
> On 11/23/2016 01:55 AM, David Howells wrote:
>> Add a system call to make extended file information available, including
>> file creation and some attribute flags where available through the
>> underlying filesystem.
>> 
>> 
>> 
>> OVERVIEW
>> 
>> 
>> The idea was initially proposed as a set of xattrs that could be retrieved
>> with getxattr(), but the general preferance proved to be for a new syscall
> 
> s/preferance/preference/
> 
>> with an extended stat structure.
>> 
>> This can feasibly be used to support a number of things, not all of which
>> are added here:
> 
> It would be very useful if this overview distinguishes which of the features
> below are supported in the initial implementation, versus which features
> (e.g., femtosecond timestamps) are simply allowed for in a future
> implementation.
> 
>> (1) Better support for the y2038 problem [Arnd Bergmann].
>> 
>> (2) Creation time: The SMB protocol carries the creation time, which could
>> be exported by Samba, which will in turn help CIFS make use of
>> FS-Cache as that can be used for coherency data.
>> 
>> This is also specified in NFSv4 as a recommended attribute and could
>> be exported by NFSD [Steve French].
>> 
>> (3) Lightweight stat: Ask for just those details of interest, and allow a
>> netfs (such as NFS) to approximate anything not of interest, possibly
>> without going to the server [Trond Myklebust, Ulrich Drepper, Andreas
>> Dilger].
>> 
>> (4) Heavyweight stat: Force a netfs to go to the server, even if it thinks
>> its cached attributes are up to date [Trond Myklebust].
>> 
>> (5) Data version number: Could be used by userspace NFS servers [Aneesh
>> Kumar].
>> 
>> Can also be used to modify fill_post_wcc() in NFSD which retrieves
>> i_version directly, but has just called vfs_getattr().  It could get
>> it from the kstat struct if it used vfs_xgetattr() instead.
>> 
>> (6) BSD stat compatibility: Including more fields from the BSD stat such
>> as creation time (st_btime) and inode generation number (st_gen)
>> [Jeremy Allison, Bernd Schubert].
>> 
>> (7) Inode generation number: Useful for FUSE and userspace NFS servers
>> [Bernd Schubert].  This was asked for but later deemed unnecessary
>> with the open-by-handle capability available
>> 
>> (8) Extra coherency data may be useful in making backups [Andreas Dilger].
> 
> Can you elaborate on the point [8] in this commit message. It's not clear
> to me at least what this is about.

To be honest, I'm not even sure what this means, and I've looked through
a few years of different threads trying to find what I suggested that
lead to this entry (looks like statx was first posted on June 29, 2010)
and this entry first appeared in v6 of the patch on July 14, 2010.  The
only comment that seems related is:

https://www.spinics.net/lists/linux-fsdevel/msg33884.html

where I was just pointing out that i_version is updated for regular files,
which is really the same as #5 above. I'm fine with removing it.

Cheers, Andreas

>> 
>> (9) Allow the filesystem to indicate what it can/cannot provide: A
>> filesystem can now say it doesn't support a standard stat feature if
>> that isn't available, so if, for instance, inode numbers or UIDs don't
>> exist or are fabricated locally...
>> 
>> (10) Make the fields a consistent size on all arches and make them large.
>> 
>> (11) Store a 16-byte volume ID in the superblock that can be returned in
>> struct xstat [Steve French].
>> 
>> (12) Include granularity fields in the time data to indicate the
>> granularity of each of the times (NFSv4 time_delta) [Steve French].
>> 
>> (13) FS_IOC_GETFLAGS value.  These could be translated to BSD's st_flags.
>> Note that the Linux IOC flags are a mess and filesystems such as Ext4
>> define flags that aren't in linux/fs.h, so translation in the kernel
>> may be a necessity (or, possibly, we provide the filesystem type too).
>> 
>> (14) Mask of features available on file (eg: ACLs, seclabel) [Brad Boyer,
>> Michael Kerrisk].
>> 
>> (15) Spare space, request flags and information flags are provided for
>> future expansion.
>> 
>> (16) Femtosecond-resolution timestamps [Dave Chinner].
>> 
>> 
>> ===
>> NEW SYSTEM CALL
>> ===
>> 
>> The new system call is:
>> 
>>  int ret = statx(int dfd,
>>  const char *filename,
>>  unsigned int flags,
> 
> In the 0/4 of this patch series, this argument is called 'atflags'.
> These should be consistent. 'flags' seems correct to me.
> 
>>  unsigned int mask,
>>  struct statx *buffer);
>> 
>> The dfd, filename and flags parameters indicate the file to query, in a
>> similar way to fstatat().  There is no equivalent of lstat() as that can be
>> emulated with 

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Andreas Dilger]

On Nov 23, 2016, at 1:37 AM, Michael Kerrisk  wrote:
> 
> Hi David,
> 
> On 11/23/2016 01:55 AM, David Howells wrote:
>> Add a system call to make extended file information available, including
>> file creation and some attribute flags where available through the
>> underlying filesystem.
>> 
>> 
>> 
>> OVERVIEW
>> 
>> 
>> The idea was initially proposed as a set of xattrs that could be retrieved
>> with getxattr(), but the general preferance proved to be for a new syscall
> 
> s/preferance/preference/
> 
>> with an extended stat structure.
>> 
>> This can feasibly be used to support a number of things, not all of which
>> are added here:
> 
> It would be very useful if this overview distinguishes which of the features
> below are supported in the initial implementation, versus which features
> (e.g., femtosecond timestamps) are simply allowed for in a future
> implementation.
> 
>> (1) Better support for the y2038 problem [Arnd Bergmann].
>> 
>> (2) Creation time: The SMB protocol carries the creation time, which could
>> be exported by Samba, which will in turn help CIFS make use of
>> FS-Cache as that can be used for coherency data.
>> 
>> This is also specified in NFSv4 as a recommended attribute and could
>> be exported by NFSD [Steve French].
>> 
>> (3) Lightweight stat: Ask for just those details of interest, and allow a
>> netfs (such as NFS) to approximate anything not of interest, possibly
>> without going to the server [Trond Myklebust, Ulrich Drepper, Andreas
>> Dilger].
>> 
>> (4) Heavyweight stat: Force a netfs to go to the server, even if it thinks
>> its cached attributes are up to date [Trond Myklebust].
>> 
>> (5) Data version number: Could be used by userspace NFS servers [Aneesh
>> Kumar].
>> 
>> Can also be used to modify fill_post_wcc() in NFSD which retrieves
>> i_version directly, but has just called vfs_getattr().  It could get
>> it from the kstat struct if it used vfs_xgetattr() instead.
>> 
>> (6) BSD stat compatibility: Including more fields from the BSD stat such
>> as creation time (st_btime) and inode generation number (st_gen)
>> [Jeremy Allison, Bernd Schubert].
>> 
>> (7) Inode generation number: Useful for FUSE and userspace NFS servers
>> [Bernd Schubert].  This was asked for but later deemed unnecessary
>> with the open-by-handle capability available
>> 
>> (8) Extra coherency data may be useful in making backups [Andreas Dilger].
> 
> Can you elaborate on the point [8] in this commit message. It's not clear
> to me at least what this is about.

To be honest, I'm not even sure what this means, and I've looked through
a few years of different threads trying to find what I suggested that
lead to this entry (looks like statx was first posted on June 29, 2010)
and this entry first appeared in v6 of the patch on July 14, 2010.  The
only comment that seems related is:

https://www.spinics.net/lists/linux-fsdevel/msg33884.html

where I was just pointing out that i_version is updated for regular files,
which is really the same as #5 above. I'm fine with removing it.

Cheers, Andreas

>> 
>> (9) Allow the filesystem to indicate what it can/cannot provide: A
>> filesystem can now say it doesn't support a standard stat feature if
>> that isn't available, so if, for instance, inode numbers or UIDs don't
>> exist or are fabricated locally...
>> 
>> (10) Make the fields a consistent size on all arches and make them large.
>> 
>> (11) Store a 16-byte volume ID in the superblock that can be returned in
>> struct xstat [Steve French].
>> 
>> (12) Include granularity fields in the time data to indicate the
>> granularity of each of the times (NFSv4 time_delta) [Steve French].
>> 
>> (13) FS_IOC_GETFLAGS value.  These could be translated to BSD's st_flags.
>> Note that the Linux IOC flags are a mess and filesystems such as Ext4
>> define flags that aren't in linux/fs.h, so translation in the kernel
>> may be a necessity (or, possibly, we provide the filesystem type too).
>> 
>> (14) Mask of features available on file (eg: ACLs, seclabel) [Brad Boyer,
>> Michael Kerrisk].
>> 
>> (15) Spare space, request flags and information flags are provided for
>> future expansion.
>> 
>> (16) Femtosecond-resolution timestamps [Dave Chinner].
>> 
>> 
>> ===
>> NEW SYSTEM CALL
>> ===
>> 
>> The new system call is:
>> 
>>  int ret = statx(int dfd,
>>  const char *filename,
>>  unsigned int flags,
> 
> In the 0/4 of this patch series, this argument is called 'atflags'.
> These should be consistent. 'flags' seems correct to me.
> 
>>  unsigned int mask,
>>  struct statx *buffer);
>> 
>> The dfd, filename and flags parameters indicate the file to query, in a
>> similar way to fstatat().  There is no equivalent of lstat() as that can be
>> emulated with statx() by passing 

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Michael Kerrisk (man-pages)]

Hi David,

On 11/23/2016 01:55 AM, David Howells wrote:
> Add a system call to make extended file information available, including
> file creation and some attribute flags where available through the
> underlying filesystem.
> 
> 
> 
> OVERVIEW
> 
> 
> The idea was initially proposed as a set of xattrs that could be retrieved
> with getxattr(), but the general preferance proved to be for a new syscall

s/preferance/preference/

> with an extended stat structure.
> 
> This can feasibly be used to support a number of things, not all of which
> are added here:

It would be very useful if this overview distinguishes which of the features
below are supported in the initial implementation, versus which features
(e.g., femtosecond timestamps) are simply allowed for in a future
implementation.

>  (1) Better support for the y2038 problem [Arnd Bergmann].
> 
>  (2) Creation time: The SMB protocol carries the creation time, which could
>  be exported by Samba, which will in turn help CIFS make use of
>  FS-Cache as that can be used for coherency data.
> 
>  This is also specified in NFSv4 as a recommended attribute and could
>  be exported by NFSD [Steve French].
> 
>  (3) Lightweight stat: Ask for just those details of interest, and allow a
>  netfs (such as NFS) to approximate anything not of interest, possibly
>  without going to the server [Trond Myklebust, Ulrich Drepper, Andreas
>  Dilger].
> 
>  (4) Heavyweight stat: Force a netfs to go to the server, even if it thinks
>  its cached attributes are up to date [Trond Myklebust].
> 
>  (5) Data version number: Could be used by userspace NFS servers [Aneesh
>  Kumar].
> 
>  Can also be used to modify fill_post_wcc() in NFSD which retrieves
>  i_version directly, but has just called vfs_getattr().  It could get
>  it from the kstat struct if it used vfs_xgetattr() instead.
> 
>  (6) BSD stat compatibility: Including more fields from the BSD stat such
>  as creation time (st_btime) and inode generation number (st_gen)
>  [Jeremy Allison, Bernd Schubert].
> 
>  (7) Inode generation number: Useful for FUSE and userspace NFS servers
>  [Bernd Schubert].  This was asked for but later deemed unnecessary
>  with the open-by-handle capability available
> 
>  (8) Extra coherency data may be useful in making backups [Andreas Dilger].

Can you elaborate on the point [8] in this commit message. It's not clear
to me at least what this is about.
> 
>  (9) Allow the filesystem to indicate what it can/cannot provide: A
>  filesystem can now say it doesn't support a standard stat feature if
>  that isn't available, so if, for instance, inode numbers or UIDs don't
>  exist or are fabricated locally...
> 
> (10) Make the fields a consistent size on all arches and make them large.
> 
> (11) Store a 16-byte volume ID in the superblock that can be returned in
>  struct xstat [Steve French].
> 
> (12) Include granularity fields in the time data to indicate the
>  granularity of each of the times (NFSv4 time_delta) [Steve French].
> 
> (13) FS_IOC_GETFLAGS value.  These could be translated to BSD's st_flags.
>  Note that the Linux IOC flags are a mess and filesystems such as Ext4
>  define flags that aren't in linux/fs.h, so translation in the kernel
>  may be a necessity (or, possibly, we provide the filesystem type too).
> 
> (14) Mask of features available on file (eg: ACLs, seclabel) [Brad Boyer,
>  Michael Kerrisk].
> 
> (15) Spare space, request flags and information flags are provided for
>  future expansion.
> 
> (16) Femtosecond-resolution timestamps [Dave Chinner].
> 
> 
> ===
> NEW SYSTEM CALL
> ===
> 
> The new system call is:
> 
>   int ret = statx(int dfd,
>   const char *filename,
>   unsigned int flags,

In the 0/4 of this patch series, this argument is called 'atflags'.
These should be consistent. 'flags' seems correct to me.

>   unsigned int mask,
>   struct statx *buffer);
> 
> The dfd, filename and flags parameters indicate the file to query, in a
> similar way to fstatat().  There is no equivalent of lstat() as that can be
> emulated with statx() by passing AT_SYMLINK_NOFOLLOW in flags.  There is
> also no equivalent of fstat() as that can be emulated by passing a NULL
> filename to statx() with the fd of interest in dfd.
> 
> Whether or not statx() synchronises the attributes with the backing store
> can be controlled (this typically only affects network filesystems) can be
> set by OR'ing a value into the flags argument:

s/can be set//

> 
>  (1) AT_STATX_SYNC_AS_STAT tells statx() to behave as stat() does in this
>  respect.
> 
>  (2) AT_STATX_FORCE_SYNC will require a network filesystem to synchronise
>  its attributes with the server - which might require data writeback to
>  occur to get the timestamps correct.
> 
>  (3) 

Re: [PATCH 1/4] statx: Add a system call to make enhanced file info available [ver #3]

[Michael Kerrisk (man-pages)]

Hi David,

On 11/23/2016 01:55 AM, David Howells wrote:
> Add a system call to make extended file information available, including
> file creation and some attribute flags where available through the
> underlying filesystem.
> 
> 
> 
> OVERVIEW
> 
> 
> The idea was initially proposed as a set of xattrs that could be retrieved
> with getxattr(), but the general preferance proved to be for a new syscall

s/preferance/preference/

> with an extended stat structure.
> 
> This can feasibly be used to support a number of things, not all of which
> are added here:

It would be very useful if this overview distinguishes which of the features
below are supported in the initial implementation, versus which features
(e.g., femtosecond timestamps) are simply allowed for in a future
implementation.

>  (1) Better support for the y2038 problem [Arnd Bergmann].
> 
>  (2) Creation time: The SMB protocol carries the creation time, which could
>  be exported by Samba, which will in turn help CIFS make use of
>  FS-Cache as that can be used for coherency data.
> 
>  This is also specified in NFSv4 as a recommended attribute and could
>  be exported by NFSD [Steve French].
> 
>  (3) Lightweight stat: Ask for just those details of interest, and allow a
>  netfs (such as NFS) to approximate anything not of interest, possibly
>  without going to the server [Trond Myklebust, Ulrich Drepper, Andreas
>  Dilger].
> 
>  (4) Heavyweight stat: Force a netfs to go to the server, even if it thinks
>  its cached attributes are up to date [Trond Myklebust].
> 
>  (5) Data version number: Could be used by userspace NFS servers [Aneesh
>  Kumar].
> 
>  Can also be used to modify fill_post_wcc() in NFSD which retrieves
>  i_version directly, but has just called vfs_getattr().  It could get
>  it from the kstat struct if it used vfs_xgetattr() instead.
> 
>  (6) BSD stat compatibility: Including more fields from the BSD stat such
>  as creation time (st_btime) and inode generation number (st_gen)
>  [Jeremy Allison, Bernd Schubert].
> 
>  (7) Inode generation number: Useful for FUSE and userspace NFS servers
>  [Bernd Schubert].  This was asked for but later deemed unnecessary
>  with the open-by-handle capability available
> 
>  (8) Extra coherency data may be useful in making backups [Andreas Dilger].

Can you elaborate on the point [8] in this commit message. It's not clear
to me at least what this is about.
> 
>  (9) Allow the filesystem to indicate what it can/cannot provide: A
>  filesystem can now say it doesn't support a standard stat feature if
>  that isn't available, so if, for instance, inode numbers or UIDs don't
>  exist or are fabricated locally...
> 
> (10) Make the fields a consistent size on all arches and make them large.
> 
> (11) Store a 16-byte volume ID in the superblock that can be returned in
>  struct xstat [Steve French].
> 
> (12) Include granularity fields in the time data to indicate the
>  granularity of each of the times (NFSv4 time_delta) [Steve French].
> 
> (13) FS_IOC_GETFLAGS value.  These could be translated to BSD's st_flags.
>  Note that the Linux IOC flags are a mess and filesystems such as Ext4
>  define flags that aren't in linux/fs.h, so translation in the kernel
>  may be a necessity (or, possibly, we provide the filesystem type too).
> 
> (14) Mask of features available on file (eg: ACLs, seclabel) [Brad Boyer,
>  Michael Kerrisk].
> 
> (15) Spare space, request flags and information flags are provided for
>  future expansion.
> 
> (16) Femtosecond-resolution timestamps [Dave Chinner].
> 
> 
> ===
> NEW SYSTEM CALL
> ===
> 
> The new system call is:
> 
>   int ret = statx(int dfd,
>   const char *filename,
>   unsigned int flags,

In the 0/4 of this patch series, this argument is called 'atflags'.
These should be consistent. 'flags' seems correct to me.

>   unsigned int mask,
>   struct statx *buffer);
> 
> The dfd, filename and flags parameters indicate the file to query, in a
> similar way to fstatat().  There is no equivalent of lstat() as that can be
> emulated with statx() by passing AT_SYMLINK_NOFOLLOW in flags.  There is
> also no equivalent of fstat() as that can be emulated by passing a NULL
> filename to statx() with the fd of interest in dfd.
> 
> Whether or not statx() synchronises the attributes with the backing store
> can be controlled (this typically only affects network filesystems) can be
> set by OR'ing a value into the flags argument:

s/can be set//

> 
>  (1) AT_STATX_SYNC_AS_STAT tells statx() to behave as stat() does in this
>  respect.
> 
>  (2) AT_STATX_FORCE_SYNC will require a network filesystem to synchronise
>  its attributes with the server - which might require data writeback to
>  occur to get the timestamps correct.
> 
>  (3)