Hi,
On 2020/8/27 22:43, Chuck Lever wrote:
> Hello!
>
>> On Aug 27, 2020, at 3:02 AM, Hou Tao <houtao1@xxxxxxxxxx> wrote:
>>
>> Don't call trace_nfsd_deleg_none() if read delegation is given,
>> else two exclusive traces will be printed:
>>
>> nfsd_deleg_open: client 5f45b854:e6058001 stateid 00000030:00000001
>> nfsd_deleg_none: client 5f45b854:e6058001 stateid 0000002f:00000001
>
> These are reporting two different state IDs: the first is a delegation
> state ID, and the second is an open state ID.
>
> So in the "no delegation" case, we want to see just the open state ID.
> In the "delegation" case, we do want to see both.
>
Thanks for your explanation.
> You could argue (successfully) that the names of the tracepoints are
> pretty lousy. Maybe better to rename:
>
> nfsd_deleg_open -> nfsd_deleg_read
> nfsd_deleg_none -> nfsd_open
>
> What do you think?
>
After the renaming, the trace looks clearer. I will do it in v2.
Thanks
>> Fix it by calling trace_nfsd_deleg_none() directly in appropriate
>> places instead of calling it by checking the value of op_delegate_type.
>>
>> Also remove the unnecessary assignment "status = nfs_ok", because
>> we can ensure status will be nfs_ok after the call of
>> nfs4_inc_and_copy_stateid().
>>
>> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
>> ---
>> fs/nfsd/nfs4state.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index c09a2a4281ec9..2e6376af701ff 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -5131,6 +5131,8 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open,
>> nfs4_put_stid(&dp->dl_stid);
>> return;
>> out_no_deleg:
>> + trace_nfsd_deleg_none(&stp->st_stid.sc_stateid);
>> +
>> open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE;
>> if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS &&
>> open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE) {
>> @@ -5232,7 +5234,8 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
>> if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
>> open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
>> open->op_why_no_deleg = WND4_NOT_WANTED;
>> - goto nodeleg;
>> + trace_nfsd_deleg_none(&stp->st_stid.sc_stateid);
>> + goto out;
>> }
>> }
>>
>> @@ -5241,9 +5244,6 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
>> * OPEN succeeds even if we fail.
>> */
>> nfs4_open_delegation(current_fh, open, stp);
>> -nodeleg:
>> - status = nfs_ok;
>> - trace_nfsd_deleg_none(&stp->st_stid.sc_stateid);
>> out:
>> /* 4.1 client trying to upgrade/downgrade delegation? */
>> if (open->op_delegate_type == NFS4_OPEN_DELEGATE_NONE && dp &&
>> --
>> 2.25.0.4.g0ad7144999
>>
>
> --
> Chuck Lever
>
>
>
> .
>
