[PATCH 0/2] NFSD: recall write delegation on GETATTR conflict

[PATCH 0/2] NFSD: recall write delegation on GETATTR conflict

[Dai Ngo]

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.

[PATCH 1/2] NFSD: handle GETATTR conflict with write delegation

[Dai Ngo]

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

[PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR

[Dai Ngo]

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

Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation

[Chuck Lever]

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
> 

Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation

[dai.ngo]

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
>>

Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation

[dai.ngo]

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
>>>

Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation

[Chuck Lever]

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
> > > 

Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation

[dai.ngo]

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
>>>>

Re: [PATCH 1/2] NFSD: handle GETATTR conflict with write delegation

[Chuck Lever]

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
> > > > > 

Re: [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR

[kernel test robot]

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

Re: [PATCH 2/2] NFSD: add counter for write delegation recall due to conflict with GETATTR

[kernel test robot]

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