2022-08-05 18:37:22

by Jeffrey Layton

[permalink] [raw]
Subject: [RFC PATCH 0/4] vfs: allow querying i_version via statx

Recently I posted a patch to turn on the i_version counter
unconditionally in ext4, and Lukas rightly pointed out that we don't
currently have an easy way to validate its functionality. You can fetch
it via NFS (and see it in network traces), but there's no way to get to
it from userland.

Besides testing, this may also be of use for userland NFS servers, or by
any program that wants to accurately check for file changes, and not be
subject to mtime granularity problems.

Comments and suggestions welcome. I'm not 100% convinced that this is a
great idea, but we've had people ask for it before and it seems like a
reasonable thing to provide.

Jeff Layton (4):
vfs: report change attribute in statx for IS_I_VERSION inodes
nfs: report the change attribute if requested
afs: fill out change attribute in statx replies
ceph: fill in the change attribute in statx requests

fs/afs/inode.c | 2 ++
fs/ceph/inode.c | 14 +++++++++-----
fs/nfs/inode.c | 3 +++
fs/stat.c | 7 +++++++
include/linux/stat.h | 1 +
include/uapi/linux/stat.h | 3 ++-
samples/vfs/test-statx.c | 4 +++-
7 files changed, 27 insertions(+), 7 deletions(-)

--
2.37.1


2022-08-05 18:37:26

by Jeffrey Layton

[permalink] [raw]
Subject: [RFC PATCH 3/4] afs: fill out change attribute in statx replies

Always copy the change attribute in a statx reply, and set the
STATX_CHGATTR bit unconditionally.

Signed-off-by: Jeff Layton <[email protected]>
---
fs/afs/inode.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 64dab70d4a4f..dffd6edd6628 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -760,6 +760,8 @@ int afs_getattr(struct user_namespace *mnt_userns, const struct path *path,
do {
read_seqbegin_or_lock(&vnode->cb_lock, &seq);
generic_fillattr(&init_user_ns, inode, stat);
+ stat->chgattr = inode_peek_iversion_raw(inode);
+ stat->result_mask |= STATX_CHGATTR;
if (test_bit(AFS_VNODE_SILLY_DELETED, &vnode->flags) &&
stat->nlink > 0)
stat->nlink -= 1;
--
2.37.1

2022-08-05 18:37:59

by Jeffrey Layton

[permalink] [raw]
Subject: [RFC PATCH 4/4] ceph: fill in the change attribute in statx requests

When statx requests the change attribute, request the full gamut of caps
(similarly to how ctime is handled). When the change attribute seems to
be valid, return it in the chgattr field.

Signed-off-by: Jeff Layton <[email protected]>
---
fs/ceph/inode.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 56c53ab3618e..fb2ed85f9083 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2408,10 +2408,10 @@ static int statx_to_caps(u32 want, umode_t mode)
{
int mask = 0;

- if (want & (STATX_MODE|STATX_UID|STATX_GID|STATX_CTIME|STATX_BTIME))
+ if (want & (STATX_MODE|STATX_UID|STATX_GID|STATX_CTIME|STATX_BTIME|STATX_CHGATTR))
mask |= CEPH_CAP_AUTH_SHARED;

- if (want & (STATX_NLINK|STATX_CTIME)) {
+ if (want & (STATX_NLINK|STATX_CTIME|STATX_CHGATTR)) {
/*
* The link count for directories depends on inode->i_subdirs,
* and that is only updated when Fs caps are held.
@@ -2422,11 +2422,10 @@ static int statx_to_caps(u32 want, umode_t mode)
mask |= CEPH_CAP_LINK_SHARED;
}

- if (want & (STATX_ATIME|STATX_MTIME|STATX_CTIME|STATX_SIZE|
- STATX_BLOCKS))
+ if (want & (STATX_ATIME|STATX_MTIME|STATX_CTIME|STATX_SIZE| STATX_BLOCKS|STATX_CHGATTR))
mask |= CEPH_CAP_FILE_SHARED;

- if (want & (STATX_CTIME))
+ if (want & (STATX_CTIME|STATX_CHGATTR))
mask |= CEPH_CAP_XATTR_SHARED;

return mask;
@@ -2468,6 +2467,11 @@ int ceph_getattr(struct user_namespace *mnt_userns, const struct path *path,
valid_mask |= STATX_BTIME;
}

+ if (request_mask & STATX_CHGATTR) {
+ stat->chgattr = inode_peek_iversion_raw(inode);
+ valid_mask |= STATX_CHGATTR;
+ }
+
if (ceph_snap(inode) == CEPH_NOSNAP)
stat->dev = inode->i_sb->s_dev;
else
--
2.37.1

2022-08-05 19:00:04

by Frank Filz

[permalink] [raw]
Subject: RE: [RFC PATCH 0/4] vfs: allow querying i_version via statx

> Recently I posted a patch to turn on the i_version counter unconditionally
in
> ext4, and Lukas rightly pointed out that we don't currently have an easy
way to
> validate its functionality. You can fetch it via NFS (and see it in
network traces),
> but there's no way to get to it from userland.
>
> Besides testing, this may also be of use for userland NFS servers, or by
any
> program that wants to accurately check for file changes, and not be
subject to
> mtime granularity problems.

This would definitely be useful for NFS Ganesha.

Thanks

Frank

> Comments and suggestions welcome. I'm not 100% convinced that this is a
> great idea, but we've had people ask for it before and it seems like a
reasonable
> thing to provide.
>
> Jeff Layton (4):
> vfs: report change attribute in statx for IS_I_VERSION inodes
> nfs: report the change attribute if requested
> afs: fill out change attribute in statx replies
> ceph: fill in the change attribute in statx requests
>
> fs/afs/inode.c | 2 ++
> fs/ceph/inode.c | 14 +++++++++-----
> fs/nfs/inode.c | 3 +++
> fs/stat.c | 7 +++++++
> include/linux/stat.h | 1 +
> include/uapi/linux/stat.h | 3 ++-
> samples/vfs/test-statx.c | 4 +++-
> 7 files changed, 27 insertions(+), 7 deletions(-)
>
> --
> 2.37.1

2022-08-08 12:06:28

by Xiubo Li

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] ceph: fill in the change attribute in statx requests


On 8/6/22 2:35 AM, Jeff Layton wrote:
> When statx requests the change attribute, request the full gamut of caps
> (similarly to how ctime is handled). When the change attribute seems to
> be valid, return it in the chgattr field.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/ceph/inode.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 56c53ab3618e..fb2ed85f9083 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -2408,10 +2408,10 @@ static int statx_to_caps(u32 want, umode_t mode)
> {
> int mask = 0;
>
> - if (want & (STATX_MODE|STATX_UID|STATX_GID|STATX_CTIME|STATX_BTIME))
> + if (want & (STATX_MODE|STATX_UID|STATX_GID|STATX_CTIME|STATX_BTIME|STATX_CHGATTR))
> mask |= CEPH_CAP_AUTH_SHARED;
>
> - if (want & (STATX_NLINK|STATX_CTIME)) {
> + if (want & (STATX_NLINK|STATX_CTIME|STATX_CHGATTR)) {
> /*
> * The link count for directories depends on inode->i_subdirs,
> * and that is only updated when Fs caps are held.
> @@ -2422,11 +2422,10 @@ static int statx_to_caps(u32 want, umode_t mode)
> mask |= CEPH_CAP_LINK_SHARED;
> }
>
> - if (want & (STATX_ATIME|STATX_MTIME|STATX_CTIME|STATX_SIZE|
> - STATX_BLOCKS))
> + if (want & (STATX_ATIME|STATX_MTIME|STATX_CTIME|STATX_SIZE| STATX_BLOCKS|STATX_CHGATTR))
> mask |= CEPH_CAP_FILE_SHARED;
>
> - if (want & (STATX_CTIME))
> + if (want & (STATX_CTIME|STATX_CHGATTR))
> mask |= CEPH_CAP_XATTR_SHARED;
>
> return mask;
> @@ -2468,6 +2467,11 @@ int ceph_getattr(struct user_namespace *mnt_userns, const struct path *path,
> valid_mask |= STATX_BTIME;
> }
>
> + if (request_mask & STATX_CHGATTR) {
> + stat->chgattr = inode_peek_iversion_raw(inode);
> + valid_mask |= STATX_CHGATTR;
> + }
> +
> if (ceph_snap(inode) == CEPH_NOSNAP)
> stat->dev = inode->i_sb->s_dev;
> else

Reviewed-by: Xiubo Li <[email protected]>