| Message ID | 1685500507-23598-2-git-send-email-dai.ngo@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFSD: recall write delegation on GETATTR conflict | expand |
On Tue, 2023-05-30 at 19:35 -0700, Dai Ngo wrote: > If the GETATTR request on a file that has write delegation in effect > and the request attributes include the change info and size attribute > then the write delegation is recalled. If the delegation is returned > within 30ms then the GETATTR is serviced as normal otherwise the > NFS4ERR_DELAY error is returned for the GETATTR. > > Signed-off-by: Dai Ngo <dai.ngo@oracle.com> > --- > fs/nfsd/nfs4state.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > fs/nfsd/nfs4xdr.c | 5 +++++ > fs/nfsd/state.h | 3 +++ > 3 files changed, 68 insertions(+) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index b90b74a5e66e..fea78d90ecf7 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -8353,3 +8353,63 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate, > { > get_stateid(cstate, &u->write.wr_stateid); > } > + > +/** > + * nfsd4_deleg_getattr_conflict - Trigger recall if GETATTR causes conflict > + * @rqstp: RPC transaction context > + * @inode: file to be checked for a conflict > + * > + * This function is called when there is a conflict between a write > + * delegation and a change/size GETATR from another client. The server > + * must either use the CB_GETATTR to get the current values of the > + * attributes from the client that hold the delegation or recall the > + * delegation before replying to the GETATTR. See RFC 8881 section > + * 18.7.4. > + * > + * Returns 0 if there is no conflict; otherwise an nfs_stat > + * code is returned. > + */ > +__be32 > +nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct inode *inode) > +{ > + __be32 status; > + struct file_lock_context *ctx; > + struct file_lock *fl; > + struct nfs4_delegation *dp; > + > + ctx = locks_inode_context(inode); > + if (!ctx) > + return 0; > + spin_lock(&ctx->flc_lock); > + list_for_each_entry(fl, &ctx->flc_lease, fl_list) { > + if (fl->fl_flags == FL_LAYOUT) > + continue; > + if (fl->fl_lmops != &nfsd_lease_mng_ops) { > + /* > + * non-nfs lease, if it's a lease with F_RDLCK then > + * we are done; there isn't any write delegation > + * on this inode > + */ > + if (fl->fl_type == F_RDLCK) > + break; > + goto break_lease; > + } > + if (fl->fl_type == F_WRLCK) { > + dp = fl->fl_owner; > + if (dp->dl_recall.cb_clp == *(rqstp->rq_lease_breaker)) { > + spin_unlock(&ctx->flc_lock); > + return 0; > + } > +break_lease: > + spin_unlock(&ctx->flc_lock); > + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); > + if (status != nfserr_jukebox || > + !nfsd_wait_for_delegreturn(rqstp, inode)) > + return status; > + return 0; > + } > + break; > + } > + spin_unlock(&ctx->flc_lock); > + return 0; > +} I think you have the logic right now, but this function kind of resembles spaghetti. I'll add a reviewed-by, but it might be good to simplify this function somehow later. > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index b83954fc57e3..4590b893dbc8 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -2970,6 +2970,11 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp, > if (status) > goto out; > } > + if (bmval0 & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) { > + status = nfsd4_deleg_getattr_conflict(rqstp, d_inode(dentry)); > + if (status) > + goto out; > + } > > err = vfs_getattr(&path, &stat, > STATX_BASIC_STATS | STATX_BTIME | STATX_CHANGE_COOKIE, > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > index d49d3060ed4f..cbddcf484dba 100644 > --- a/fs/nfsd/state.h > +++ b/fs/nfsd/state.h > @@ -732,4 +732,7 @@ static inline bool try_to_expire_client(struct nfs4_client *clp) > cmpxchg(&clp->cl_state, NFSD4_COURTESY, NFSD4_EXPIRABLE); > return clp->cl_state == NFSD4_EXPIRABLE; > } > + > +extern __be32 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, > + struct inode *inode); > #endif /* NFSD4_STATE_H */ Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index b90b74a5e66e..fea78d90ecf7 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -8353,3 +8353,63 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate, { get_stateid(cstate, &u->write.wr_stateid); } + +/** + * nfsd4_deleg_getattr_conflict - Trigger recall if GETATTR causes conflict + * @rqstp: RPC transaction context + * @inode: file to be checked for a conflict + * + * This function is called when there is a conflict between a write + * delegation and a change/size GETATR from another client. The server + * must either use the CB_GETATTR to get the current values of the + * attributes from the client that hold the delegation or recall the + * delegation before replying to the GETATTR. See RFC 8881 section + * 18.7.4. + * + * Returns 0 if there is no conflict; otherwise an nfs_stat + * code is returned. + */ +__be32 +nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct inode *inode) +{ + __be32 status; + struct file_lock_context *ctx; + struct file_lock *fl; + struct nfs4_delegation *dp; + + ctx = locks_inode_context(inode); + if (!ctx) + return 0; + spin_lock(&ctx->flc_lock); + list_for_each_entry(fl, &ctx->flc_lease, fl_list) { + if (fl->fl_flags == FL_LAYOUT) + continue; + if (fl->fl_lmops != &nfsd_lease_mng_ops) { + /* + * non-nfs lease, if it's a lease with F_RDLCK then + * we are done; there isn't any write delegation + * on this inode + */ + if (fl->fl_type == F_RDLCK) + break; + goto break_lease; + } + if (fl->fl_type == F_WRLCK) { + dp = fl->fl_owner; + if (dp->dl_recall.cb_clp == *(rqstp->rq_lease_breaker)) { + spin_unlock(&ctx->flc_lock); + return 0; + } +break_lease: + spin_unlock(&ctx->flc_lock); + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); + if (status != nfserr_jukebox || + !nfsd_wait_for_delegreturn(rqstp, inode)) + return status; + return 0; + } + break; + } + spin_unlock(&ctx->flc_lock); + return 0; +} diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index b83954fc57e3..4590b893dbc8 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -2970,6 +2970,11 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp, if (status) goto out; } + if (bmval0 & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) { + status = nfsd4_deleg_getattr_conflict(rqstp, d_inode(dentry)); + if (status) + goto out; + } err = vfs_getattr(&path, &stat, STATX_BASIC_STATS | STATX_BTIME | STATX_CHANGE_COOKIE, diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index d49d3060ed4f..cbddcf484dba 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -732,4 +732,7 @@ static inline bool try_to_expire_client(struct nfs4_client *clp) cmpxchg(&clp->cl_state, NFSD4_COURTESY, NFSD4_EXPIRABLE); return clp->cl_state == NFSD4_EXPIRABLE; } + +extern __be32 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, + struct inode *inode); #endif /* NFSD4_STATE_H */
If the GETATTR request on a file that has write delegation in effect and the request attributes include the change info and size attribute then the write delegation is recalled. If the delegation is returned within 30ms then the GETATTR is serviced as normal otherwise the NFS4ERR_DELAY error is returned for the GETATTR. Signed-off-by: Dai Ngo <dai.ngo@oracle.com> --- fs/nfsd/nfs4state.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++ fs/nfsd/nfs4xdr.c | 5 +++++ fs/nfsd/state.h | 3 +++ 3 files changed, 68 insertions(+)