* [PATCH 0/2] NFSD: recall write delegation on GETATTR conflict @ 2023-05-26 17:38 Dai Ngo 2023-05-26 17:38 ` [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation Dai Ngo 2023-05-26 17:38 ` [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR Dai Ngo 0 siblings, 2 replies; 11+ messages in thread From: Dai Ngo @ 2023-05-26 17:38 UTC (permalink / raw) To: chuck.lever, jlayton; +Cc: linux-nfs, linux-fsdevel This patch series adds the recall of write delegation when there is conflict with a GETATTR and a counter in /proc/net/rpc/nfsd to keep count of this recall. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation 2023-05-26 17:38 [PATCH 0/2] NFSD: recall write delegation on GETATTR conflict Dai Ngo @ 2023-05-26 17:38 ` Dai Ngo 2023-05-26 18:38 ` Chuck Lever 2023-05-26 17:38 ` [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR Dai Ngo 1 sibling, 1 reply; 11+ messages in thread From: Dai Ngo @ 2023-05-26 17:38 UTC (permalink / raw) To: chuck.lever, jlayton; +Cc: linux-nfs, linux-fsdevel 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. The server waits a maximum of 90ms for the delegation to be returned before replying NFS4ERR_DELAY for the GETATTR. Signed-off-by: Dai Ngo <dai.ngo@oracle.com> --- fs/nfsd/nfs4state.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ fs/nfsd/nfs4xdr.c | 5 +++++ fs/nfsd/state.h | 3 +++ 3 files changed, 56 insertions(+) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index b90b74a5e66e..9f551dbf50d6 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -8353,3 +8353,51 @@ 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 + * + * 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; + int cnt; + 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 || + fl->fl_lmops != &nfsd_lease_mng_ops) + continue; + 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; + } + spin_unlock(&ctx->flc_lock); + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); + if (status != nfserr_jukebox) + return status; + for (cnt = 3; cnt > 0; --cnt) { + if (!nfsd_wait_for_delegreturn(rqstp, inode)) + continue; + return 0; + } + return status; + } + 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 */ -- 2.9.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation 2023-05-26 17:38 ` [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation Dai Ngo @ 2023-05-26 18:38 ` Chuck Lever 2023-05-26 19:34 ` dai.ngo 0 siblings, 1 reply; 11+ messages in thread From: Chuck Lever @ 2023-05-26 18:38 UTC (permalink / raw) To: Dai Ngo; +Cc: chuck.lever, jlayton, linux-nfs, linux-fsdevel On Fri, May 26, 2023 at 10:38:41AM -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. The server waits a maximum of > 90ms for the delegation to be returned before replying NFS4ERR_DELAY > for the GETATTR. > > Signed-off-by: Dai Ngo <dai.ngo@oracle.com> > --- > fs/nfsd/nfs4state.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > fs/nfsd/nfs4xdr.c | 5 +++++ > fs/nfsd/state.h | 3 +++ > 3 files changed, 56 insertions(+) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index b90b74a5e66e..9f551dbf50d6 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -8353,3 +8353,51 @@ 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 > + * Let's have this comment explain why this is necessary. At the least, it needs to cite RFC 8881 Section 18.7.4, which REQUIREs a conflicting write delegation to be gone before the server can respond to a change/size GETATTR request. > + * 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; > + int cnt; > + 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 || > + fl->fl_lmops != &nfsd_lease_mng_ops) > + continue; > + 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; > + } > + spin_unlock(&ctx->flc_lock); > + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); > + if (status != nfserr_jukebox) > + return status; > + for (cnt = 3; cnt > 0; --cnt) { > + if (!nfsd_wait_for_delegreturn(rqstp, inode)) > + continue; > + return 0; > + } I'd rather not retry here. Can you can say why a 30ms wait is not sufficient for this case? > + return status; > + } > + 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 */ > -- > 2.9.5 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation 2023-05-26 18:38 ` Chuck Lever @ 2023-05-26 19:34 ` dai.ngo 2023-05-26 19:36 ` dai.ngo 2023-05-26 19:40 ` Chuck Lever 0 siblings, 2 replies; 11+ messages in thread From: dai.ngo @ 2023-05-26 19:34 UTC (permalink / raw) To: Chuck Lever; +Cc: chuck.lever, jlayton, linux-nfs, linux-fsdevel On 5/26/23 11:38 AM, Chuck Lever wrote: > On Fri, May 26, 2023 at 10:38:41AM -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. The server waits a maximum of >> 90ms for the delegation to be returned before replying NFS4ERR_DELAY >> for the GETATTR. >> >> Signed-off-by: Dai Ngo <dai.ngo@oracle.com> >> --- >> fs/nfsd/nfs4state.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ >> fs/nfsd/nfs4xdr.c | 5 +++++ >> fs/nfsd/state.h | 3 +++ >> 3 files changed, 56 insertions(+) >> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index b90b74a5e66e..9f551dbf50d6 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -8353,3 +8353,51 @@ 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 >> + * > Let's have this comment explain why this is necessary. At the least, > it needs to cite RFC 8881 Section 18.7.4, which REQUIREs a conflicting > write delegation to be gone before the server can respond to a > change/size GETATTR request. ok, will add the comment. > > >> + * 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; >> + int cnt; >> + 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 || >> + fl->fl_lmops != &nfsd_lease_mng_ops) >> + continue; >> + 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; >> + } >> + spin_unlock(&ctx->flc_lock); >> + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); >> + if (status != nfserr_jukebox) >> + return status; >> + for (cnt = 3; cnt > 0; --cnt) { >> + if (!nfsd_wait_for_delegreturn(rqstp, inode)) >> + continue; >> + return 0; >> + } > I'd rather not retry here. Can you can say why a 30ms wait is not > sufficient for this case? on my VMs, it takes about 80ms for the the delegation return to complete. -Dai > > >> + return status; >> + } >> + 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 */ >> -- >> 2.9.5 >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation 2023-05-26 19:34 ` dai.ngo @ 2023-05-26 19:36 ` dai.ngo 2023-05-26 19:40 ` Chuck Lever 1 sibling, 0 replies; 11+ messages in thread From: dai.ngo @ 2023-05-26 19:36 UTC (permalink / raw) To: Chuck Lever; +Cc: chuck.lever, jlayton, linux-nfs, linux-fsdevel On 5/26/23 12:34 PM, dai.ngo@oracle.com wrote: > > On 5/26/23 11:38 AM, Chuck Lever wrote: >> On Fri, May 26, 2023 at 10:38:41AM -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. The server waits a maximum of >>> 90ms for the delegation to be returned before replying NFS4ERR_DELAY >>> for the GETATTR. >>> >>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com> >>> --- >>> fs/nfsd/nfs4state.c | 48 >>> ++++++++++++++++++++++++++++++++++++++++++++++++ >>> fs/nfsd/nfs4xdr.c | 5 +++++ >>> fs/nfsd/state.h | 3 +++ >>> 3 files changed, 56 insertions(+) >>> >>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >>> index b90b74a5e66e..9f551dbf50d6 100644 >>> --- a/fs/nfsd/nfs4state.c >>> +++ b/fs/nfsd/nfs4state.c >>> @@ -8353,3 +8353,51 @@ 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 >>> + * >> Let's have this comment explain why this is necessary. At the least, >> it needs to cite RFC 8881 Section 18.7.4, which REQUIREs a conflicting >> write delegation to be gone before the server can respond to a >> change/size GETATTR request. > > ok, will add the comment. > >> >> >>> + * 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; >>> + int cnt; >>> + 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 || >>> + fl->fl_lmops != &nfsd_lease_mng_ops) >>> + continue; >>> + 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; >>> + } >>> + spin_unlock(&ctx->flc_lock); >>> + status = nfserrno(nfsd_open_break_lease(inode, >>> NFSD_MAY_READ)); >>> + if (status != nfserr_jukebox) >>> + return status; >>> + for (cnt = 3; cnt > 0; --cnt) { >>> + if (!nfsd_wait_for_delegreturn(rqstp, inode)) >>> + continue; >>> + return 0; >>> + } >> I'd rather not retry here. Can you can say why a 30ms wait is not >> sufficient for this case? > > on my VMs, it takes about 80ms for the the delegation return to complete. Otherwise it takes about 180ms for the CB_RECALL and DELEGRETURN to complete before the client can get a successful reply of the GETATTR. -Dai > > -Dai > >> >> >>> + return status; >>> + } >>> + 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 */ >>> -- >>> 2.9.5 >>> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation 2023-05-26 19:34 ` dai.ngo 2023-05-26 19:36 ` dai.ngo @ 2023-05-26 19:40 ` Chuck Lever 2023-05-26 20:54 ` dai.ngo 1 sibling, 1 reply; 11+ messages in thread From: Chuck Lever @ 2023-05-26 19:40 UTC (permalink / raw) To: dai.ngo; +Cc: chuck.lever, jlayton, linux-nfs, linux-fsdevel On Fri, May 26, 2023 at 12:34:16PM -0700, dai.ngo@oracle.com wrote: > > On 5/26/23 11:38 AM, Chuck Lever wrote: > > On Fri, May 26, 2023 at 10:38:41AM -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. The server waits a maximum of > > > 90ms for the delegation to be returned before replying NFS4ERR_DELAY > > > for the GETATTR. > > > > > > Signed-off-by: Dai Ngo <dai.ngo@oracle.com> > > > --- > > > fs/nfsd/nfs4state.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > > > fs/nfsd/nfs4xdr.c | 5 +++++ > > > fs/nfsd/state.h | 3 +++ > > > 3 files changed, 56 insertions(+) > > > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > > index b90b74a5e66e..9f551dbf50d6 100644 > > > --- a/fs/nfsd/nfs4state.c > > > +++ b/fs/nfsd/nfs4state.c > > > @@ -8353,3 +8353,51 @@ 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 > > > + * > > Let's have this comment explain why this is necessary. At the least, > > it needs to cite RFC 8881 Section 18.7.4, which REQUIREs a conflicting > > write delegation to be gone before the server can respond to a > > change/size GETATTR request. > > ok, will add the comment. > > > > > > > > + * 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; > > > + int cnt; > > > + 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 || > > > + fl->fl_lmops != &nfsd_lease_mng_ops) > > > + continue; > > > + 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; > > > + } > > > + spin_unlock(&ctx->flc_lock); > > > + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); > > > + if (status != nfserr_jukebox) > > > + return status; > > > + for (cnt = 3; cnt > 0; --cnt) { > > > + if (!nfsd_wait_for_delegreturn(rqstp, inode)) > > > + continue; > > > + return 0; > > > + } > > I'd rather not retry here. Can you can say why a 30ms wait is not > > sufficient for this case? > > on my VMs, it takes about 80ms for the the delegation return to complete. I'd rather not tune for tiny VM guests. How long does it take for a native client to handle CB_RECALL and return the delegation? It shouldn't take longer to do so than it would for the other cases the server already handles in under 30ms. Even 30ms is a long time to hold up an nfsd thread, IMO. > > > + return status; > > > + } > > > + 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 */ > > > -- > > > 2.9.5 > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation 2023-05-26 19:40 ` Chuck Lever @ 2023-05-26 20:54 ` dai.ngo 2023-05-26 23:27 ` Chuck Lever 0 siblings, 1 reply; 11+ messages in thread From: dai.ngo @ 2023-05-26 20:54 UTC (permalink / raw) To: Chuck Lever; +Cc: chuck.lever, jlayton, linux-nfs, linux-fsdevel On 5/26/23 12:40 PM, Chuck Lever wrote: > On Fri, May 26, 2023 at 12:34:16PM -0700, dai.ngo@oracle.com wrote: >> On 5/26/23 11:38 AM, Chuck Lever wrote: >>> On Fri, May 26, 2023 at 10:38:41AM -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. The server waits a maximum of >>>> 90ms for the delegation to be returned before replying NFS4ERR_DELAY >>>> for the GETATTR. >>>> >>>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com> >>>> --- >>>> fs/nfsd/nfs4state.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ >>>> fs/nfsd/nfs4xdr.c | 5 +++++ >>>> fs/nfsd/state.h | 3 +++ >>>> 3 files changed, 56 insertions(+) >>>> >>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >>>> index b90b74a5e66e..9f551dbf50d6 100644 >>>> --- a/fs/nfsd/nfs4state.c >>>> +++ b/fs/nfsd/nfs4state.c >>>> @@ -8353,3 +8353,51 @@ 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 >>>> + * >>> Let's have this comment explain why this is necessary. At the least, >>> it needs to cite RFC 8881 Section 18.7.4, which REQUIREs a conflicting >>> write delegation to be gone before the server can respond to a >>> change/size GETATTR request. >> ok, will add the comment. >> >>> >>>> + * 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; >>>> + int cnt; >>>> + 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 || >>>> + fl->fl_lmops != &nfsd_lease_mng_ops) >>>> + continue; >>>> + 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; >>>> + } >>>> + spin_unlock(&ctx->flc_lock); >>>> + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); >>>> + if (status != nfserr_jukebox) >>>> + return status; >>>> + for (cnt = 3; cnt > 0; --cnt) { >>>> + if (!nfsd_wait_for_delegreturn(rqstp, inode)) >>>> + continue; >>>> + return 0; >>>> + } >>> I'd rather not retry here. Can you can say why a 30ms wait is not >>> sufficient for this case? >> on my VMs, it takes about 80ms for the the delegation return to complete. > I'd rather not tune for tiny VM guests. How long does it take for a > native client to handle CB_RECALL and return the delegation? It > shouldn't take longer to do so than it would for the other cases the > server already handles in under 30ms. > > Even 30ms is a long time to hold up an nfsd thread, IMO. If the client takes less than 30ms to return the delegation then the server will reply to the GETATTR right away, it does not wait for the whole 90ms. The 90ms is for the worst case scenario where the client/network is slow or under load. Even if the server waits for the whole 90ms it's still faster to reply to the GETATTR than sending CB_RECALL and wait for DELEGRETURN before the server can reply to the GETATTR. -Dai > > >>>> + return status; >>>> + } >>>> + 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 */ >>>> -- >>>> 2.9.5 >>>> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation 2023-05-26 20:54 ` dai.ngo @ 2023-05-26 23:27 ` Chuck Lever 0 siblings, 0 replies; 11+ messages in thread From: Chuck Lever @ 2023-05-26 23:27 UTC (permalink / raw) To: dai.ngo; +Cc: chuck.lever, jlayton, linux-nfs, linux-fsdevel On Fri, May 26, 2023 at 01:54:12PM -0700, dai.ngo@oracle.com wrote: > > On 5/26/23 12:40 PM, Chuck Lever wrote: > > On Fri, May 26, 2023 at 12:34:16PM -0700, dai.ngo@oracle.com wrote: > > > On 5/26/23 11:38 AM, Chuck Lever wrote: > > > > On Fri, May 26, 2023 at 10:38:41AM -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. The server waits a maximum of > > > > > 90ms for the delegation to be returned before replying NFS4ERR_DELAY > > > > > for the GETATTR. > > > > > > > > > > Signed-off-by: Dai Ngo <dai.ngo@oracle.com> > > > > > --- > > > > > fs/nfsd/nfs4state.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > > > > > fs/nfsd/nfs4xdr.c | 5 +++++ > > > > > fs/nfsd/state.h | 3 +++ > > > > > 3 files changed, 56 insertions(+) > > > > > > > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > > > > index b90b74a5e66e..9f551dbf50d6 100644 > > > > > --- a/fs/nfsd/nfs4state.c > > > > > +++ b/fs/nfsd/nfs4state.c > > > > > @@ -8353,3 +8353,51 @@ 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 > > > > > + * > > > > Let's have this comment explain why this is necessary. At the least, > > > > it needs to cite RFC 8881 Section 18.7.4, which REQUIREs a conflicting > > > > write delegation to be gone before the server can respond to a > > > > change/size GETATTR request. > > > ok, will add the comment. > > > > > > > > > > > > + * 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; > > > > > + int cnt; > > > > > + 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 || > > > > > + fl->fl_lmops != &nfsd_lease_mng_ops) > > > > > + continue; > > > > > + 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; > > > > > + } > > > > > + spin_unlock(&ctx->flc_lock); > > > > > + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); > > > > > + if (status != nfserr_jukebox) > > > > > + return status; > > > > > + for (cnt = 3; cnt > 0; --cnt) { > > > > > + if (!nfsd_wait_for_delegreturn(rqstp, inode)) > > > > > + continue; > > > > > + return 0; > > > > > + } > > > > I'd rather not retry here. Can you can say why a 30ms wait is not > > > > sufficient for this case? > > > on my VMs, it takes about 80ms for the the delegation return to complete. > > I'd rather not tune for tiny VM guests. How long does it take for a > > native client to handle CB_RECALL and return the delegation? It > > shouldn't take longer to do so than it would for the other cases the > > server already handles in under 30ms. > > > > Even 30ms is a long time to hold up an nfsd thread, IMO. > > If the client takes less than 30ms to return the delegation then the > server will reply to the GETATTR right away, it does not wait for the > whole 90ms. > > The 90ms is for the worst case scenario where the client/network is slow > or under load. Even if the server waits for the whole 90ms it's still > faster to reply to the GETATTR than sending CB_RECALL and wait for > DELEGRETURN before the server can reply to the GETATTR. The reason for the short timeout is we can't tie up nfsd threads for a long time; that can amount to denial of service. I'm not concerned about a single slow client, but enough clients that don't respond quickly to CB_RECALL can prevent the server from making forward progress, even for a short period, and that will be noticeable. In Linux, generally we optimize for the fastest case, not the slow cases like this one. Make the fast clients as fast as possible; do not penalize everyone for the slow cases. So, please make this function call nfsd_wait_for_delegreturn() only once, and leave NFSD_DELEGRETURN_TIMEOUT at 30ms. > > > > > + return status; > > > > > + } > > > > > + 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 */ > > > > > -- > > > > > 2.9.5 > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR 2023-05-26 17:38 [PATCH 0/2] NFSD: recall write delegation on GETATTR conflict Dai Ngo 2023-05-26 17:38 ` [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation Dai Ngo @ 2023-05-26 17:38 ` Dai Ngo 2023-05-27 11:58 ` kernel test robot 2023-05-27 17:15 ` kernel test robot 1 sibling, 2 replies; 11+ messages in thread From: Dai Ngo @ 2023-05-26 17:38 UTC (permalink / raw) To: chuck.lever, jlayton; +Cc: linux-nfs, linux-fsdevel Add counter to keep track of how many times write delegations are recalled due to conflict with GETATTR. Signed-off-by: Dai Ngo <dai.ngo@oracle.com> --- fs/nfsd/nfs4state.c | 1 + fs/nfsd/stats.c | 2 ++ fs/nfsd/stats.h | 5 +++++ 3 files changed, 8 insertions(+) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 9f551dbf50d6..89ec251f7e83 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -8386,6 +8386,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct inode *inode) return 0; } spin_unlock(&ctx->flc_lock); + nfsd_stats_wdeleg_getattr_inc(); status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); if (status != nfserr_jukebox) return status; diff --git a/fs/nfsd/stats.c b/fs/nfsd/stats.c index 777e24e5da33..63797635e1c3 100644 --- a/fs/nfsd/stats.c +++ b/fs/nfsd/stats.c @@ -65,6 +65,8 @@ static int nfsd_show(struct seq_file *seq, void *v) seq_printf(seq, " %lld", percpu_counter_sum_positive(&nfsdstats.counter[NFSD_STATS_NFS4_OP(i)])); } + seq_printf(seq, "\nwdeleg_getattr %lld", + percpu_counter_sum_positive(&nfsdstats.counter[NFSD_STATS_WDELEG_GETATTR])); seq_putc(seq, '\n'); #endif diff --git a/fs/nfsd/stats.h b/fs/nfsd/stats.h index 9b43dc3d9991..e31bd3abdf07 100644 --- a/fs/nfsd/stats.h +++ b/fs/nfsd/stats.h @@ -22,6 +22,7 @@ enum { NFSD_STATS_FIRST_NFS4_OP, /* count of individual nfsv4 operations */ NFSD_STATS_LAST_NFS4_OP = NFSD_STATS_FIRST_NFS4_OP + LAST_NFS4_OP, #define NFSD_STATS_NFS4_OP(op) (NFSD_STATS_FIRST_NFS4_OP + (op)) + NFSD_STATS_WDELEG_GETATTR, /* count of getattr conflict with wdeleg */ #endif NFSD_STATS_COUNTERS_NUM }; @@ -93,4 +94,8 @@ static inline void nfsd_stats_drc_mem_usage_sub(struct nfsd_net *nn, s64 amount) percpu_counter_sub(&nn->counter[NFSD_NET_DRC_MEM_USAGE], amount); } +static inline void nfsd_stats_wdeleg_getattr_inc(void) +{ + percpu_counter_inc(&nfsdstats.counter[NFSD_STATS_WDELEG_GETATTR]); +} #endif /* _NFSD_STATS_H */ -- 2.9.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR 2023-05-26 17:38 ` [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR Dai Ngo @ 2023-05-27 11:58 ` kernel test robot 2023-05-27 17:15 ` kernel test robot 1 sibling, 0 replies; 11+ messages in thread From: kernel test robot @ 2023-05-27 11:58 UTC (permalink / raw) To: Dai Ngo, chuck.lever, jlayton; +Cc: oe-kbuild-all, linux-nfs, linux-fsdevel Hi Dai, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.4-rc3 next-20230525] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Dai-Ngo/NFSD-handle-GETATTR-conflict-with-write-delegation/20230527-013936 base: linus/master patch link: https://lore.kernel.org/r/1685122722-18287-3-git-send-email-dai.ngo%40oracle.com patch subject: [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR config: parisc-defconfig (https://download.01.org/0day-ci/archive/20230527/202305271936.3kL7Ufxk-lkp@intel.com/config) compiler: hppa-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): mkdir -p ~/bin wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/a90d0ca71c9459b76f9faa8c704c029ac8066d00 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Dai-Ngo/NFSD-handle-GETATTR-conflict-with-write-delegation/20230527-013936 git checkout a90d0ca71c9459b76f9faa8c704c029ac8066d00 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross W=1 O=build_dir ARCH=parisc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash fs/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202305271936.3kL7Ufxk-lkp@intel.com/ All errors (new ones prefixed by >>): In file included from fs/nfsd/nfsd.h:28, from fs/nfsd/state.h:42, from fs/nfsd/xdr4.h:40, from fs/nfsd/trace.h:17, from fs/nfsd/trace.c:4: fs/nfsd/stats.h: In function 'nfsd_stats_wdeleg_getattr_inc': >> fs/nfsd/stats.h:99:47: error: 'NFSD_STATS_WDELEG_GETATTR' undeclared (first use in this function) 99 | percpu_counter_inc(&nfsdstats.counter[NFSD_STATS_WDELEG_GETATTR]); | ^~~~~~~~~~~~~~~~~~~~~~~~~ fs/nfsd/stats.h:99:47: note: each undeclared identifier is reported only once for each function it appears in -- In file included from fs/nfsd/nfsd.h:28, from fs/nfsd/export.c:21: fs/nfsd/stats.h: In function 'nfsd_stats_wdeleg_getattr_inc': >> fs/nfsd/stats.h:99:47: error: 'NFSD_STATS_WDELEG_GETATTR' undeclared (first use in this function) 99 | percpu_counter_inc(&nfsdstats.counter[NFSD_STATS_WDELEG_GETATTR]); | ^~~~~~~~~~~~~~~~~~~~~~~~~ fs/nfsd/stats.h:99:47: note: each undeclared identifier is reported only once for each function it appears in fs/nfsd/export.c: In function 'exp_rootfh': fs/nfsd/export.c:1005:34: warning: variable 'inode' set but not used [-Wunused-but-set-variable] 1005 | struct inode *inode; | ^~~~~ -- In file included from fs/nfsd/nfsd.h:28, from fs/nfsd/state.h:42, from fs/nfsd/xdr4.h:40, from fs/nfsd/trace.h:17, from fs/nfsd/trace.c:4: fs/nfsd/stats.h: In function 'nfsd_stats_wdeleg_getattr_inc': >> fs/nfsd/stats.h:99:47: error: 'NFSD_STATS_WDELEG_GETATTR' undeclared (first use in this function) 99 | percpu_counter_inc(&nfsdstats.counter[NFSD_STATS_WDELEG_GETATTR]); | ^~~~~~~~~~~~~~~~~~~~~~~~~ fs/nfsd/stats.h:99:47: note: each undeclared identifier is reported only once for each function it appears in In file included from fs/nfsd/trace.h:1589: include/trace/define_trace.h: At top level: include/trace/define_trace.h:95:42: fatal error: ./trace.h: No such file or directory 95 | #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) | ^ compilation terminated. vim +/NFSD_STATS_WDELEG_GETATTR +99 fs/nfsd/stats.h 96 97 static inline void nfsd_stats_wdeleg_getattr_inc(void) 98 { > 99 percpu_counter_inc(&nfsdstats.counter[NFSD_STATS_WDELEG_GETATTR]); -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR 2023-05-26 17:38 ` [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR Dai Ngo 2023-05-27 11:58 ` kernel test robot @ 2023-05-27 17:15 ` kernel test robot 1 sibling, 0 replies; 11+ messages in thread From: kernel test robot @ 2023-05-27 17:15 UTC (permalink / raw) To: Dai Ngo, chuck.lever, jlayton Cc: llvm, oe-kbuild-all, linux-nfs, linux-fsdevel Hi Dai, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.4-rc3 next-20230525] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Dai-Ngo/NFSD-handle-GETATTR-conflict-with-write-delegation/20230527-013936 base: linus/master patch link: https://lore.kernel.org/r/1685122722-18287-3-git-send-email-dai.ngo%40oracle.com patch subject: [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR config: i386-randconfig-i074-20230526 (https://download.01.org/0day-ci/archive/20230528/202305280121.3RAeAt4l-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): mkdir -p ~/bin wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/a90d0ca71c9459b76f9faa8c704c029ac8066d00 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Dai-Ngo/NFSD-handle-GETATTR-conflict-with-write-delegation/20230527-013936 git checkout a90d0ca71c9459b76f9faa8c704c029ac8066d00 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202305280121.3RAeAt4l-lkp@intel.com/ All errors (new ones prefixed by >>): In file included from fs/nfsd/trace.c:4: In file included from fs/nfsd/trace.h:17: In file included from fs/nfsd/xdr4.h:40: In file included from fs/nfsd/state.h:42: In file included from fs/nfsd/nfsd.h:28: >> fs/nfsd/stats.h:99:40: error: use of undeclared identifier 'NFSD_STATS_WDELEG_GETATTR' percpu_counter_inc(&nfsdstats.counter[NFSD_STATS_WDELEG_GETATTR]); ^ 1 error generated. -- In file included from fs/nfsd/export.c:21: In file included from fs/nfsd/nfsd.h:28: >> fs/nfsd/stats.h:99:40: error: use of undeclared identifier 'NFSD_STATS_WDELEG_GETATTR' percpu_counter_inc(&nfsdstats.counter[NFSD_STATS_WDELEG_GETATTR]); ^ fs/nfsd/export.c:1005:17: warning: variable 'inode' set but not used [-Wunused-but-set-variable] struct inode *inode; ^ 1 warning and 1 error generated. vim +/NFSD_STATS_WDELEG_GETATTR +99 fs/nfsd/stats.h 96 97 static inline void nfsd_stats_wdeleg_getattr_inc(void) 98 { > 99 percpu_counter_inc(&nfsdstats.counter[NFSD_STATS_WDELEG_GETATTR]); -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-05-27 17:16 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-26 17:38 [PATCH 0/2] NFSD: recall write delegation on GETATTR conflict Dai Ngo 2023-05-26 17:38 ` [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation Dai Ngo 2023-05-26 18:38 ` Chuck Lever 2023-05-26 19:34 ` dai.ngo 2023-05-26 19:36 ` dai.ngo 2023-05-26 19:40 ` Chuck Lever 2023-05-26 20:54 ` dai.ngo 2023-05-26 23:27 ` Chuck Lever 2023-05-26 17:38 ` [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR Dai Ngo 2023-05-27 11:58 ` kernel test robot 2023-05-27 17:15 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).