| Message ID | 20240617012137.674046-12-trondmy@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | OPEN optimisations and Attribute delegations | expand |
On Sun, 2024-06-16 at 21:21 -0400, trondmy@kernel.org wrote: > From: Trond Myklebust <trond.myklebust@primarydata.com> > > If the atime or mtime attributes were delegated, then we need to > propagate their new values back to the server when returning the > delegation. > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > Signed-off-by: Lance Shelton <lance.shelton@hammerspace.com> > Reviewed-by: Jeff Layton <jlayton@kernel.org> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > 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 d9117630e062..d5edb3b3eeef 100644 > --- a/fs/nfs/delegation.c > +++ b/fs/nfs/delegation.c > @@ -266,7 +266,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; > @@ -275,9 +277,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 001551e2ab60..71524d34ed20 100644 > --- a/fs/nfs/delegation.h > +++ b/fs/nfs/delegation.h > @@ -70,7 +70,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 1209ce22158e..88edeaf5b5d5 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -6739,7 +6739,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); > @@ -6791,12 +6794,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); I'm seeing the client try to set bad atimes with newly-created files. To reproduce, I just created a file with vim on an NFS mount and wrote a few bytes to it and then closed it. The client gets a WRITE_DELEG_ATTRS delegation, and after I close the file it does a SETATTR before the DELEGRETURN for TIME_DELEG_ACCESS and TIME_DELEG_MODIFY. The mtime looks fine, but the atime is ~1500 seconds after the epoch. pcap is attached. Cheers, > + 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; > @@ -6814,13 +6832,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:
diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index d9117630e062..d5edb3b3eeef 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -266,7 +266,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; @@ -275,9 +277,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 001551e2ab60..71524d34ed20 100644 --- a/fs/nfs/delegation.h +++ b/fs/nfs/delegation.h @@ -70,7 +70,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 1209ce22158e..88edeaf5b5d5 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -6739,7 +6739,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); @@ -6791,12 +6794,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; @@ -6814,13 +6832,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: