From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
Signed-off-by: Lance Shelton <[email protected]>
---
fs/nfs/delegation.c | 9 +++++----
fs/nfs/delegation.h | 4 +++-
fs/nfs/nfs4proc.c | 27 ++++++++++++++++++++++++---
3 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index e72eead06c08..d8f4a1cdbc8e 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -258,7 +258,9 @@ void nfs_inode_reclaim_delegation(struct inode *inode, const struct cred *cred,
}
}
-static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)
+static int nfs_do_return_delegation(struct inode *inode,
+ struct nfs_delegation *delegation,
+ int issync)
{
const struct cred *cred;
int res = 0;
@@ -267,9 +269,8 @@ static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *
spin_lock(&delegation->lock);
cred = get_cred(delegation->cred);
spin_unlock(&delegation->lock);
- res = nfs4_proc_delegreturn(inode, cred,
- &delegation->stateid,
- issync);
+ res = nfs4_proc_delegreturn(inode, cred, &delegation->stateid,
+ delegation, issync);
put_cred(cred);
}
return res;
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index 2e9ad83acf2c..da910e2e98a4 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -72,7 +72,9 @@ void nfs_test_expired_all_delegations(struct nfs_client *clp);
void nfs_reap_expired_delegations(struct nfs_client *clp);
/* NFSv4 delegation-related procedures */
-int nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred, const nfs4_stateid *stateid, int issync);
+int nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred,
+ const nfs4_stateid *stateid,
+ struct nfs_delegation *delegation, int issync);
int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid);
int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state, const nfs4_stateid *stateid);
bool nfs4_copy_delegation_stateid(struct inode *inode, fmode_t flags, nfs4_stateid *dst, const struct cred **cred);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 140ff1d75320..b0c1564a7bc7 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6718,7 +6718,10 @@ static const struct rpc_call_ops nfs4_delegreturn_ops = {
.rpc_release = nfs4_delegreturn_release,
};
-static int _nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred, const nfs4_stateid *stateid, int issync)
+static int _nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred,
+ const nfs4_stateid *stateid,
+ struct nfs_delegation *delegation,
+ int issync)
{
struct nfs4_delegreturndata *data;
struct nfs_server *server = NFS_SERVER(inode);
@@ -6770,12 +6773,27 @@ static int _nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred,
}
}
+ if (delegation &&
+ test_bit(NFS_DELEGATION_DELEGTIME, &delegation->flags)) {
+ if (delegation->type & FMODE_READ) {
+ data->sattr.atime = inode_get_atime(inode);
+ data->sattr.atime_set = true;
+ }
+ if (delegation->type & FMODE_WRITE) {
+ data->sattr.mtime = inode_get_mtime(inode);
+ data->sattr.mtime_set = true;
+ }
+ data->args.sattr_args = &data->sattr;
+ data->res.sattr_res = true;
+ }
+
if (!data->inode)
nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1,
1);
else
nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1,
0);
+
task_setup_data.callback_data = data;
msg.rpc_argp = &data->args;
msg.rpc_resp = &data->res;
@@ -6793,13 +6811,16 @@ static int _nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred,
return status;
}
-int nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred, const nfs4_stateid *stateid, int issync)
+int nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred,
+ const nfs4_stateid *stateid,
+ struct nfs_delegation *delegation, int issync)
{
struct nfs_server *server = NFS_SERVER(inode);
struct nfs4_exception exception = { };
int err;
do {
- err = _nfs4_proc_delegreturn(inode, cred, stateid, issync);
+ err = _nfs4_proc_delegreturn(inode, cred, stateid,
+ delegation, issync);
trace_nfs4_delegreturn(inode, stateid, err);
switch (err) {
case -NFS4ERR_STALE_STATEID:
--
2.45.2
From: Trond Myklebust <[email protected]>
nfs_setattr calls nfs_update_inode() directly, so we have to reset the
m/ctime there.
Signed-off-by: Trond Myklebust <[email protected]>
Signed-off-by: Lance Shelton <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/inode.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 91c0aeaf6c1e..e03c512c8535 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -605,6 +605,46 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
}
EXPORT_SYMBOL_GPL(nfs_fhget);
+static void
+nfs_fattr_fixup_delegated(struct inode *inode, struct nfs_fattr *fattr)
+{
+ unsigned long cache_validity = NFS_I(inode)->cache_validity;
+
+ if (!nfs_have_read_or_write_delegation(inode))
+ return;
+
+ if (!(cache_validity & NFS_INO_REVAL_FORCED))
+ cache_validity &= ~(NFS_INO_INVALID_ATIME
+ | NFS_INO_INVALID_CHANGE
+ | NFS_INO_INVALID_CTIME
+ | NFS_INO_INVALID_MTIME
+ | NFS_INO_INVALID_SIZE);
+
+ if (!(cache_validity & NFS_INO_INVALID_SIZE))
+ fattr->valid &= ~(NFS_ATTR_FATTR_PRESIZE
+ | NFS_ATTR_FATTR_SIZE);
+
+ if (!(cache_validity & NFS_INO_INVALID_CHANGE))
+ fattr->valid &= ~(NFS_ATTR_FATTR_PRECHANGE
+ | NFS_ATTR_FATTR_CHANGE);
+
+ if (nfs_have_delegated_mtime(inode)) {
+ if (!(cache_validity & NFS_INO_INVALID_CTIME))
+ fattr->valid &= ~(NFS_ATTR_FATTR_PRECTIME
+ | NFS_ATTR_FATTR_CTIME);
+
+ if (!(cache_validity & NFS_INO_INVALID_MTIME))
+ fattr->valid &= ~(NFS_ATTR_FATTR_PREMTIME
+ | NFS_ATTR_FATTR_MTIME);
+
+ if (!(cache_validity & NFS_INO_INVALID_ATIME))
+ fattr->valid &= ~NFS_ATTR_FATTR_ATIME;
+ } else if (nfs_have_delegated_atime(inode)) {
+ if (!(cache_validity & NFS_INO_INVALID_ATIME))
+ fattr->valid &= ~NFS_ATTR_FATTR_ATIME;
+ }
+}
+
void nfs_update_delegated_atime(struct inode *inode)
{
spin_lock(&inode->i_lock);
@@ -2163,6 +2203,9 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
*/
nfsi->read_cache_jiffies = fattr->time_start;
+ /* Fix up any delegated attributes in the struct nfs_fattr */
+ nfs_fattr_fixup_delegated(inode, fattr);
+
save_cache_validity = nfsi->cache_validity;
nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
| NFS_INO_INVALID_ATIME
--
2.45.2
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
Signed-off-by: Lance Shelton <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index b0c1564a7bc7..512268c732a1 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -310,6 +310,18 @@ static void nfs4_bitmap_copy_adjust(__u32 *dst, const __u32 *src,
dst[1] &= ~FATTR4_WORD1_MODE;
if (!(cache_validity & NFS_INO_INVALID_OTHER))
dst[1] &= ~(FATTR4_WORD1_OWNER | FATTR4_WORD1_OWNER_GROUP);
+
+ if (nfs_have_delegated_mtime(inode)) {
+ if (!(cache_validity & NFS_INO_INVALID_ATIME))
+ dst[1] &= ~FATTR4_WORD1_TIME_ACCESS;
+ if (!(cache_validity & NFS_INO_INVALID_MTIME))
+ dst[1] &= ~FATTR4_WORD1_TIME_MODIFY;
+ if (!(cache_validity & NFS_INO_INVALID_CTIME))
+ dst[1] &= ~FATTR4_WORD1_TIME_METADATA;
+ } else if (nfs_have_delegated_atime(inode)) {
+ if (!(cache_validity & NFS_INO_INVALID_ATIME))
+ dst[1] &= ~FATTR4_WORD1_TIME_ACCESS;
+ }
}
static void nfs4_setup_readdir(u64 cookie, __be32 *verifier, struct dentry *dentry,
@@ -3414,7 +3426,8 @@ static int nfs4_do_setattr(struct inode *inode, const struct cred *cred,
.inode = inode,
.stateid = &arg.stateid,
};
- unsigned long adjust_flags = NFS_INO_INVALID_CHANGE;
+ unsigned long adjust_flags = NFS_INO_INVALID_CHANGE |
+ NFS_INO_INVALID_CTIME;
int err;
if (sattr->ia_valid & (ATTR_MODE | ATTR_KILL_SUID | ATTR_KILL_SGID))
@@ -4958,7 +4971,7 @@ static int _nfs4_proc_link(struct inode *inode, struct inode *dir, const struct
nfs4_inode_make_writeable(inode);
nfs4_bitmap_copy_adjust(bitmask, nfs4_bitmask(server, res.fattr->label), inode,
- NFS_INO_INVALID_CHANGE);
+ NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_CTIME);
status = nfs4_call_sync(server->client, server, &msg, &arg.seq_args, &res.seq_res, 1);
if (!status) {
nfs4_update_changeattr(dir, &res.cinfo, res.fattr->time_start,
--
2.45.2
From: Trond Myklebust <[email protected]>
Query the server for the OPEN arguments that it supports so that
we can figure out which extensions we can use.
Signed-off-by: Trond Myklebust <[email protected]>
Signed-off-by: Lance Shelton <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 20 ++++++++++++++++++--
fs/nfs/nfs4xdr.c | 24 ++++++++++++++++++++++++
include/linux/nfs4.h | 2 ++
include/linux/nfs_xdr.h | 9 +++++++++
4 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 512268c732a1..ca2c115b6545 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3885,11 +3885,14 @@ static void nfs4_close_context(struct nfs_open_context *ctx, int is_sync)
#define FATTR4_WORD1_NFS40_MASK (2*FATTR4_WORD1_MOUNTED_ON_FILEID - 1UL)
#define FATTR4_WORD2_NFS41_MASK (2*FATTR4_WORD2_SUPPATTR_EXCLCREAT - 1UL)
-#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_TIME_DELEG_MODIFY - 1UL)
+#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_OPEN_ARGUMENTS - 1UL)
static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *fhandle)
{
- u32 bitmask[3] = {}, minorversion = server->nfs_client->cl_minorversion;
+ u32 minorversion = server->nfs_client->cl_minorversion;
+ u32 bitmask[3] = {
+ [0] = FATTR4_WORD0_SUPPORTED_ATTRS,
+ };
struct nfs4_server_caps_arg args = {
.fhandle = fhandle,
.bitmask = bitmask,
@@ -3915,6 +3918,14 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
status = nfs4_call_sync(server->client, server, &msg, &args.seq_args, &res.seq_res, 0);
if (status == 0) {
+ bitmask[0] = (FATTR4_WORD0_SUPPORTED_ATTRS |
+ FATTR4_WORD0_FH_EXPIRE_TYPE |
+ FATTR4_WORD0_LINK_SUPPORT |
+ FATTR4_WORD0_SYMLINK_SUPPORT |
+ FATTR4_WORD0_ACLSUPPORT |
+ FATTR4_WORD0_CASE_INSENSITIVE |
+ FATTR4_WORD0_CASE_PRESERVING) &
+ res.attr_bitmask[0];
/* Sanity check the server answers */
switch (minorversion) {
case 0:
@@ -3923,9 +3934,14 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
break;
case 1:
res.attr_bitmask[2] &= FATTR4_WORD2_NFS41_MASK;
+ bitmask[2] = FATTR4_WORD2_SUPPATTR_EXCLCREAT &
+ res.attr_bitmask[2];
break;
case 2:
res.attr_bitmask[2] &= FATTR4_WORD2_NFS42_MASK;
+ bitmask[2] = (FATTR4_WORD2_SUPPATTR_EXCLCREAT |
+ FATTR4_WORD2_OPEN_ARGUMENTS) &
+ res.attr_bitmask[2];
}
memcpy(server->attr_bitmask, res.attr_bitmask, sizeof(server->attr_bitmask));
server->caps &= ~(NFS_CAP_ACLS | NFS_CAP_HARDLINKS |
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index e160a275ad4a..98aab2c324c9 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4337,6 +4337,28 @@ static int decode_attr_xattrsupport(struct xdr_stream *xdr, uint32_t *bitmap,
return 0;
}
+static int decode_attr_open_arguments(struct xdr_stream *xdr, uint32_t *bitmap,
+ struct nfs4_open_caps *res)
+{
+ memset(res, 0, sizeof(*res));
+ if (unlikely(bitmap[2] & (FATTR4_WORD2_OPEN_ARGUMENTS - 1U)))
+ return -EIO;
+ if (likely(bitmap[2] & FATTR4_WORD2_OPEN_ARGUMENTS)) {
+ if (decode_bitmap4(xdr, res->oa_share_access, ARRAY_SIZE(res->oa_share_access)) < 0)
+ return -EIO;
+ if (decode_bitmap4(xdr, res->oa_share_deny, ARRAY_SIZE(res->oa_share_deny)) < 0)
+ return -EIO;
+ if (decode_bitmap4(xdr, res->oa_share_access_want, ARRAY_SIZE(res->oa_share_access_want)) < 0)
+ return -EIO;
+ if (decode_bitmap4(xdr, res->oa_open_claim, ARRAY_SIZE(res->oa_open_claim)) < 0)
+ return -EIO;
+ if (decode_bitmap4(xdr, res->oa_createmode, ARRAY_SIZE(res->oa_createmode)) < 0)
+ return -EIO;
+ bitmap[2] &= ~FATTR4_WORD2_OPEN_ARGUMENTS;
+ }
+ return 0;
+}
+
static int verify_attr_len(struct xdr_stream *xdr, unsigned int savep, uint32_t attrlen)
{
unsigned int attrwords = XDR_QUADLEN(attrlen);
@@ -4511,6 +4533,8 @@ static int decode_server_caps(struct xdr_stream *xdr, struct nfs4_server_caps_re
if ((status = decode_attr_exclcreat_supported(xdr, bitmap,
res->exclcreat_bitmask)) != 0)
goto xdr_error;
+ if ((status = decode_attr_open_arguments(xdr, bitmap, &res->open_caps)) != 0)
+ goto xdr_error;
status = verify_attr_len(xdr, savep, attrlen);
xdr_error:
dprintk("%s: xdr returned %d!\n", __func__, -status);
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index bc13d7f04e8d..79b23ad674c8 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -512,6 +512,7 @@ enum {
enum {
FATTR4_TIME_DELEG_ACCESS = 84,
FATTR4_TIME_DELEG_MODIFY = 85,
+ FATTR4_OPEN_ARGUMENTS = 86,
};
/*
@@ -595,6 +596,7 @@ enum {
#define FATTR4_WORD2_XATTR_SUPPORT BIT(FATTR4_XATTR_SUPPORT - 64)
#define FATTR4_WORD2_TIME_DELEG_ACCESS BIT(FATTR4_TIME_DELEG_ACCESS - 64)
#define FATTR4_WORD2_TIME_DELEG_MODIFY BIT(FATTR4_TIME_DELEG_MODIFY - 64)
+#define FATTR4_WORD2_OPEN_ARGUMENTS BIT(FATTR4_OPEN_ARGUMENTS - 64)
/* MDS threshold bitmap bits */
#define THRESHOLD_RD (1UL << 0)
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index d8cfa956d24c..af510a7ec46a 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1213,6 +1213,14 @@ struct nfs4_statfs_res {
struct nfs_fsstat *fsstat;
};
+struct nfs4_open_caps {
+ u32 oa_share_access[1];
+ u32 oa_share_deny[1];
+ u32 oa_share_access_want[1];
+ u32 oa_open_claim[1];
+ u32 oa_createmode[1];
+};
+
struct nfs4_server_caps_arg {
struct nfs4_sequence_args seq_args;
struct nfs_fh *fhandle;
@@ -1229,6 +1237,7 @@ struct nfs4_server_caps_res {
u32 fh_expire_type;
u32 case_insensitive;
u32 case_preserving;
+ struct nfs4_open_caps open_caps;
};
#define NFS4_PATHNAME_MAXCOMPONENTS 512
--
2.45.2
Hi Trond,
On Thu, Jun 13, 2024 at 12:18 AM <[email protected]> wrote:
>
> From: Trond Myklebust <[email protected]>
>
> nfs_setattr calls nfs_update_inode() directly, so we have to reset the
> m/ctime there.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> Signed-off-by: Lance Shelton <[email protected]>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/inode.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
After applying this patch I see a handful of new failures on xfstests:
generic/075, generic/086, generic/112, generic/332, generic/346,
generic/647, and generic/729. I see the first five on NFS v4.2, but
647 and 729 both fail on v4.1 in addition to v4.2.
I hope this helps!
Anna
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 91c0aeaf6c1e..e03c512c8535 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -605,6 +605,46 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
> }
> EXPORT_SYMBOL_GPL(nfs_fhget);
>
> +static void
> +nfs_fattr_fixup_delegated(struct inode *inode, struct nfs_fattr *fattr)
> +{
> + unsigned long cache_validity = NFS_I(inode)->cache_validity;
> +
> + if (!nfs_have_read_or_write_delegation(inode))
> + return;
> +
> + if (!(cache_validity & NFS_INO_REVAL_FORCED))
> + cache_validity &= ~(NFS_INO_INVALID_ATIME
> + | NFS_INO_INVALID_CHANGE
> + | NFS_INO_INVALID_CTIME
> + | NFS_INO_INVALID_MTIME
> + | NFS_INO_INVALID_SIZE);
> +
> + if (!(cache_validity & NFS_INO_INVALID_SIZE))
> + fattr->valid &= ~(NFS_ATTR_FATTR_PRESIZE
> + | NFS_ATTR_FATTR_SIZE);
> +
> + if (!(cache_validity & NFS_INO_INVALID_CHANGE))
> + fattr->valid &= ~(NFS_ATTR_FATTR_PRECHANGE
> + | NFS_ATTR_FATTR_CHANGE);
> +
> + if (nfs_have_delegated_mtime(inode)) {
> + if (!(cache_validity & NFS_INO_INVALID_CTIME))
> + fattr->valid &= ~(NFS_ATTR_FATTR_PRECTIME
> + | NFS_ATTR_FATTR_CTIME);
> +
> + if (!(cache_validity & NFS_INO_INVALID_MTIME))
> + fattr->valid &= ~(NFS_ATTR_FATTR_PREMTIME
> + | NFS_ATTR_FATTR_MTIME);
> +
> + if (!(cache_validity & NFS_INO_INVALID_ATIME))
> + fattr->valid &= ~NFS_ATTR_FATTR_ATIME;
> + } else if (nfs_have_delegated_atime(inode)) {
> + if (!(cache_validity & NFS_INO_INVALID_ATIME))
> + fattr->valid &= ~NFS_ATTR_FATTR_ATIME;
> + }
> +}
> +
> void nfs_update_delegated_atime(struct inode *inode)
> {
> spin_lock(&inode->i_lock);
> @@ -2163,6 +2203,9 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
> */
> nfsi->read_cache_jiffies = fattr->time_start;
>
> + /* Fix up any delegated attributes in the struct nfs_fattr */
> + nfs_fattr_fixup_delegated(inode, fattr);
> +
> save_cache_validity = nfsi->cache_validity;
> nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
> | NFS_INO_INVALID_ATIME
> --
> 2.45.2
>
>
On Fri, 2024-06-14 at 12:32 -0400, Anna Schumaker wrote:
> Hi Trond,
>
> On Thu, Jun 13, 2024 at 12:18 AM <[email protected]> wrote:
> >
> > From: Trond Myklebust <[email protected]>
> >
> > nfs_setattr calls nfs_update_inode() directly, so we have to reset
> > the
> > m/ctime there.
> >
> > Signed-off-by: Trond Myklebust <[email protected]>
> > Signed-off-by: Lance Shelton <[email protected]>
> > Signed-off-by: Trond Myklebust <[email protected]>
> > ---
> > fs/nfs/inode.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 43 insertions(+)
>
> After applying this patch I see a handful of new failures on
> xfstests:
> generic/075, generic/086, generic/112, generic/332, generic/346,
> generic/647, and generic/729. I see the first five on NFS v4.2, but
> 647 and 729 both fail on v4.1 in addition to v4.2.
Thanks Anna!
Yes, I think that is a consequence of the changes that were made in
commit cc7f2dae63bc ("NFS: Don't store NFS_INO_REVAL_FORCED"). I can
just remove that "if (!(cache_validity & NFS_INO_REVAL_FORCED))"
altogether with that change applied.
Version 2 will be forthcoming.
>
> I hope this helps!
> Anna
>
> >
> > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > index 91c0aeaf6c1e..e03c512c8535 100644
> > --- a/fs/nfs/inode.c
> > +++ b/fs/nfs/inode.c
> > @@ -605,6 +605,46 @@ nfs_fhget(struct super_block *sb, struct
> > nfs_fh *fh, struct nfs_fattr *fattr)
> > }
> > EXPORT_SYMBOL_GPL(nfs_fhget);
> >
> > +static void
> > +nfs_fattr_fixup_delegated(struct inode *inode, struct nfs_fattr
> > *fattr)
> > +{
> > + unsigned long cache_validity = NFS_I(inode)-
> > >cache_validity;
> > +
> > + if (!nfs_have_read_or_write_delegation(inode))
> > + return;
> > +
> > + if (!(cache_validity & NFS_INO_REVAL_FORCED))
> > + cache_validity &= ~(NFS_INO_INVALID_ATIME
> > + | NFS_INO_INVALID_CHANGE
> > + | NFS_INO_INVALID_CTIME
> > + | NFS_INO_INVALID_MTIME
> > + | NFS_INO_INVALID_SIZE);
> > +
> > + if (!(cache_validity & NFS_INO_INVALID_SIZE))
> > + fattr->valid &= ~(NFS_ATTR_FATTR_PRESIZE
> > + | NFS_ATTR_FATTR_SIZE);
> > +
> > + if (!(cache_validity & NFS_INO_INVALID_CHANGE))
> > + fattr->valid &= ~(NFS_ATTR_FATTR_PRECHANGE
> > + | NFS_ATTR_FATTR_CHANGE);
> > +
> > + if (nfs_have_delegated_mtime(inode)) {
> > + if (!(cache_validity & NFS_INO_INVALID_CTIME))
> > + fattr->valid &= ~(NFS_ATTR_FATTR_PRECTIME
> > + | NFS_ATTR_FATTR_CTIME);
> > +
> > + if (!(cache_validity & NFS_INO_INVALID_MTIME))
> > + fattr->valid &= ~(NFS_ATTR_FATTR_PREMTIME
> > + | NFS_ATTR_FATTR_MTIME);
> > +
> > + if (!(cache_validity & NFS_INO_INVALID_ATIME))
> > + fattr->valid &= ~NFS_ATTR_FATTR_ATIME;
> > + } else if (nfs_have_delegated_atime(inode)) {
> > + if (!(cache_validity & NFS_INO_INVALID_ATIME))
> > + fattr->valid &= ~NFS_ATTR_FATTR_ATIME;
> > + }
> > +}
> > +
> > void nfs_update_delegated_atime(struct inode *inode)
> > {
> > spin_lock(&inode->i_lock);
> > @@ -2163,6 +2203,9 @@ static int nfs_update_inode(struct inode
> > *inode, struct nfs_fattr *fattr)
> > */
> > nfsi->read_cache_jiffies = fattr->time_start;
> >
> > + /* Fix up any delegated attributes in the struct nfs_fattr
> > */
> > + nfs_fattr_fixup_delegated(inode, fattr);
> > +
> > save_cache_validity = nfsi->cache_validity;
> > nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
> > | NFS_INO_INVALID_ATIME
> > --
> > 2.45.2
> >
> >
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
On Fri, 2024-06-14 at 15:59 -0400, Trond Myklebust wrote:
> On Fri, 2024-06-14 at 12:32 -0400, Anna Schumaker wrote:
> > Hi Trond,
> >
> > On Thu, Jun 13, 2024 at 12:18 AM <[email protected]> wrote:
> > >
> > > From: Trond Myklebust <[email protected]>
> > >
> > > nfs_setattr calls nfs_update_inode() directly, so we have to
> > > reset
> > > the
> > > m/ctime there.
> > >
> > > Signed-off-by: Trond Myklebust <[email protected]>
> > > Signed-off-by: Lance Shelton <[email protected]>
> > > Signed-off-by: Trond Myklebust <[email protected]>
> > > ---
> > > fs/nfs/inode.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 43 insertions(+)
> >
> > After applying this patch I see a handful of new failures on
> > xfstests:
> > generic/075, generic/086, generic/112, generic/332, generic/346,
> > generic/647, and generic/729. I see the first five on NFS v4.2, but
> > 647 and 729 both fail on v4.1 in addition to v4.2.
>
> Thanks Anna!
>
> Yes, I think that is a consequence of the changes that were made in
> commit cc7f2dae63bc ("NFS: Don't store NFS_INO_REVAL_FORCED"). I can
> just remove that "if (!(cache_validity & NFS_INO_REVAL_FORCED))"
> altogether with that change applied.
Sigh... Never mind. I discovered a discrepancy between what I had in
the topic branch for these features vs. what I had in our master
branch. At some point in the last few years, I must have updated the
latter without remembering to update the former.
>
> Version 2 will be forthcoming.
>
...and it will not affect the behaviour of the regular delegations.
> >
> > I hope this helps!
> > Anna
Yes. I've checked both the patchsets that I sent out the other day, and
I believe this is the only patch which deviates from our master branch.
Again thanks!
> >
> > >
> > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > > index 91c0aeaf6c1e..e03c512c8535 100644
> > > --- a/fs/nfs/inode.c
> > > +++ b/fs/nfs/inode.c
> > > @@ -605,6 +605,46 @@ nfs_fhget(struct super_block *sb, struct
> > > nfs_fh *fh, struct nfs_fattr *fattr)
> > > }
> > > EXPORT_SYMBOL_GPL(nfs_fhget);
> > >
> > > +static void
> > > +nfs_fattr_fixup_delegated(struct inode *inode, struct nfs_fattr
> > > *fattr)
> > > +{
> > > + unsigned long cache_validity = NFS_I(inode)-
> > > > cache_validity;
> > > +
> > > + if (!nfs_have_read_or_write_delegation(inode))
> > > + return;
> > > +
> > > + if (!(cache_validity & NFS_INO_REVAL_FORCED))
> > > + cache_validity &= ~(NFS_INO_INVALID_ATIME
> > > + | NFS_INO_INVALID_CHANGE
> > > + | NFS_INO_INVALID_CTIME
> > > + | NFS_INO_INVALID_MTIME
> > > + | NFS_INO_INVALID_SIZE);
> > > +
> > > + if (!(cache_validity & NFS_INO_INVALID_SIZE))
> > > + fattr->valid &= ~(NFS_ATTR_FATTR_PRESIZE
> > > + | NFS_ATTR_FATTR_SIZE);
> > > +
> > > + if (!(cache_validity & NFS_INO_INVALID_CHANGE))
> > > + fattr->valid &= ~(NFS_ATTR_FATTR_PRECHANGE
> > > + | NFS_ATTR_FATTR_CHANGE);
> > > +
> > > + if (nfs_have_delegated_mtime(inode)) {
> > > + if (!(cache_validity & NFS_INO_INVALID_CTIME))
> > > + fattr->valid &= ~(NFS_ATTR_FATTR_PRECTIME
> > > + | NFS_ATTR_FATTR_CTIME);
> > > +
> > > + if (!(cache_validity & NFS_INO_INVALID_MTIME))
> > > + fattr->valid &= ~(NFS_ATTR_FATTR_PREMTIME
> > > + | NFS_ATTR_FATTR_MTIME);
> > > +
> > > + if (!(cache_validity & NFS_INO_INVALID_ATIME))
> > > + fattr->valid &= ~NFS_ATTR_FATTR_ATIME;
> > > + } else if (nfs_have_delegated_atime(inode)) {
> > > + if (!(cache_validity & NFS_INO_INVALID_ATIME))
> > > + fattr->valid &= ~NFS_ATTR_FATTR_ATIME;
> > > + }
> > > +}
> > > +
> > > void nfs_update_delegated_atime(struct inode *inode)
> > > {
> > > spin_lock(&inode->i_lock);
> > > @@ -2163,6 +2203,9 @@ static int nfs_update_inode(struct inode
> > > *inode, struct nfs_fattr *fattr)
> > > */
> > > nfsi->read_cache_jiffies = fattr->time_start;
> > >
> > > + /* Fix up any delegated attributes in the struct
> > > nfs_fattr
> > > */
> > > + nfs_fattr_fixup_delegated(inode, fattr);
> > > +
> > > save_cache_validity = nfsi->cache_validity;
> > > nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
> > > | NFS_INO_INVALID_ATIME
> > > --
> > > 2.45.2
> > >
> > >
>
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]