Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Dai Ngo <dai.ngo@oracle.com>
CC:     "jlayton@kernel.org" <jlayton@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v4 3/3] NFSD: add CB_RECALL_ANY tracepoints
Thread-Topic: [PATCH v4 3/3] NFSD: add CB_RECALL_ANY tracepoints
Thread-Index: AQHY9LtZeVXT8D2f3EuoKPtp4QoFca452jEAgAWc1ICAAJqpAA==
Date:   Tue, 15 Nov 2022 14:21:39 +0000
Message-ID: <A25D721B-944B-43B5-ACFB-6EB7C983B355@oracle.com>
References: <1668053831-7662-1-git-send-email-dai.ngo@oracle.com>
 <1668053831-7662-4-git-send-email-dai.ngo@oracle.com>
 <F69554A8-20BB-482E-AF8A-94F90B1081FD@oracle.com>
 <3ea87405-ea0e-1a4d-63ca-7bf89188b034@oracle.com>
In-Reply-To: <3ea87405-ea0e-1a4d-63ca-7bf89188b034@oracle.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?HXW+rGoPkTQ+cg8UpIoSqwOA7tEd/o6IMFv/euuc0Cdo71MIChiFVcASyHKt?=
 =?us-ascii?Q?NTnD58PEDOjcxSbuazVRt1eecmPDR670YzdGs3jKHhK7+HB7LyVjVUYGDk7O?=
 =?us-ascii?Q?D7Qi4wrQpYWAw2niYdcYzTZR5yqqXbeWJ7qycvMQOfezi7AIHycY/NjCbjlt?=
 =?us-ascii?Q?5Zceiq6rt1Zxf6rFz/dIP3FH6cJ23z1gnf6A0LPeCuRI9+BYHfA9LW+qJLTT?=
 =?us-ascii?Q?qL5PDcVW2ysVRWXt45wspUwoQiIW0SU07kzlT6ELZpSdFdXUq3ju55UNoY1s?=
 =?us-ascii?Q?66jZ4HTr5CpfqwT9wKfY3EqzsZPQZcbKYx6wcdbKnobq4+c5M2MGCmtZjJLN?=
 =?us-ascii?Q?9f4H2A4JC42cXcCz0kfkAgDH7PRJqYyWGuaq2LuSdbz7AkQMegOEdLS6Hk3Z?=
 =?us-ascii?Q?MSrG6xuKPFO6cNk2yl9Ol2slTCNvYi1vZvriwPKI7HrpBQdP7GL6oXMgpnka?=
 =?us-ascii?Q?ZvlDsgsIXgjGf7E7ommN4AnxeTr5/xzIHQQsEtPCRClhZmHcVzLxL+nWhkVP?=
 =?us-ascii?Q?BLokwJL25oaDT42d0E/FZmzwJeL9ESvS3oghi1PKtykelBjpyiJK8NUc5vtb?=
 =?us-ascii?Q?OCXKIsm1dZnsJDx/fMgTTKcSdZcpDT2C0bg4yBG6FdPLXeE8y61mkELSYwIs?=
 =?us-ascii?Q?Dayyg5Q6EeQg+rp1cdtWAk3WsVwpNg14W2el3IiBUgUZkaD690G6MaeEDIBD?=
 =?us-ascii?Q?+KiIgw0ZY3qt0XMgPrIlIS/xx6C84pqA+iNEBtuYYiV6JBIAKg3rkZcPvPPv?=
 =?us-ascii?Q?cN54X73qm5kJH5A/WBnVhJIruzcJHNUaUTtjhDO6Z8lPPG21l5NvqwiQu8cy?=
 =?us-ascii?Q?AJZD+Ls1MpA7lcF3gjtLGt0ZyVRumcBTfkOkPHIfkxPWOiC87NpOW6MnjkrR?=
 =?us-ascii?Q?Em/O9azLobmvqx+YNDejb8IL/FTi97/2vIm0mGI6IX4o99im6nKR24frLtbk?=
 =?us-ascii?Q?Y6LQGyW1Fnn1CADWqlHhh/o/zX37d2QrSRdzNrnNK+lgDPzkvbV/k3rQMvQd?=
 =?us-ascii?Q?ojG6H8AgQo0UVmwlVezwJDOuYY7GbM4HNRZFBG/nLuJyee3hn9oxbh+8gKbq?=
 =?us-ascii?Q?2pheibGkPeNxxAWLDsona4fk7Kt/ff9gRfNoLg7Rqlane/AQmIV+A+vw5rjt?=
 =?us-ascii?Q?t/cjLPRBQSwq1gQjZV1pLrq69ToiWna2jDjlJXX/vIX0nkdF2LEfP01AZwfv?=
 =?us-ascii?Q?ziuehc9qfVPflDPFymgFubJHx3hfz4RbihdxU9F3qJLk2B6a0c2wn/PTAv2l?=
 =?us-ascii?Q?e5uJhuIxLVub21XfrY2e/KwOu5MsR5veqllPEYY1NCm1TYw66kbdmx3uxYG9?=
 =?us-ascii?Q?d3K+eUp892Ol1x71osGBiHDMcrc5iiV0j2frmClzwGnTubUPDMD+pmQxmYdf?=
 =?us-ascii?Q?2xNtKMrU3QAb+rW1A67GxKK7hD1p1TN9ue4TdSXeDSopCD9WEEMulLdAGFt3?=
 =?us-ascii?Q?Lll9U0wuRXnnJALDlGL+mQN+uAe09t3aZY8daB7ixSILFGbjmL3wd1A5SE4C?=
 =?us-ascii?Q?wWoFIVMQiYXoAHR1H4ipLe1TfepQ507xP+D8tSydJ+oANIGIiWwgFhdNCGHX?=
 =?us-ascii?Q?jH9Nu5sohwKLfgBBK7CGHhajQ9BsTIDRkz+IRpHjNyUEimPYE39iF+xoKP7m?=
 =?us-ascii?Q?+Q=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <C5E0897BF3344F4BAFC8CB840BBCE24B@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: eQgJqq5478sfIbhMAzwYAfo03WD9p9SjIClplZDKTRyX5dBU69QjWqWakUUsDIxeAxENTWnNj53+1bp8AFAefmUV1eKkDq1TgZ0z3fiLyrPqnW4hEqgF8zej4WHRhixbwWSWnrVEMGnjZC0B4+NlqDwXWItYn0sIjqUvLblSDedZT0vH8Ozv3tZF1kXmfmBMAYA8pm3SHTay+TEPrejHf7VHseVhFQnlGgSHq5/PHKoaUzLKgYf6BXoKO31fXSFPFr4KBJ6isSmFi3AOOvMfX32J4lmSt5VM2yroxLd1jbBNDOnp5NYRYKqlQKfPsn9/fQP4Q1djwM47eJIuts0M1fwvyxizBJ21VbKhvs58gUku33B9lrFgp2/IpDrXg0AxvN7RWRryV+O/tfA4lj13vI4ExofyMqirVI5AXop285BUwOU3uYdkVzr65KUqyZwya2R8wht5M09YCxGu4hjrzWsFEgg5jAU1YQPndinFfrSigkXIeJmp236IKK4a1q6JMNCGvRuhtjKAmzA+Aw/x/+oiUDc63LPcfshNbxPk5TxnHzHiDv2F/RplnZvcESRRQTF/C59/ZQ03KAvSxpvvn1+d1oUfzfqFhoLJLwd6ggOWQTPCSpTbadYJSx6fOIlmZvElqIZQiqOZcgc2vY3VHTXd4aOIR/k3ZIEaa3g4hTlbo+3dSB92C8xbElOdF/odyBekh8u5Q+cDUe0554d5AHJdWN9ILN9znmLTfcbigelfuQBldqws6uLeI1Ba6FktM1N+TAzF52eOPegVCSzRm9zkmjtivi7XxZuJit2GXrk=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a48ddd7d-7d8d-41d1-d8c9-08dac714b624
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Nov 2022 14:21:39.5019
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 4k4BdCwCOi7D/NqmNq5VdRCAmf/SJdHvspbMdT0AVSmXLLMngCkSekX4/Wb+mF4twSqRqwrfpgS/wd6ref5s2g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4432
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-15_08,2022-11-15_03,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 bulkscore=0 spamscore=0
 mlxlogscore=999 phishscore=0 mlxscore=0 adultscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2211150096
X-Proofpoint-ORIG-GUID: KeYwJkcjwLji8ikz090o0aA2KvftzErP
X-Proofpoint-GUID: KeYwJkcjwLji8ikz090o0aA2KvftzErP
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Nov 15, 2022, at 12:08 AM, Dai Ngo <dai.ngo@oracle.com> wrote:
>=20
>=20
> On 11/11/22 7:25 AM, Chuck Lever III wrote:
>>=20
>>> On Nov 9, 2022, at 11:17 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>>=20
>>> Add tracepoints to trace start and end of CB_RECALL_ANY operation.
>>>=20
>>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>>> ---
>>> fs/nfsd/nfs4state.c |  2 ++
>>> fs/nfsd/trace.h     | 53 ++++++++++++++++++++++++++++++++++++++++++++++=
+++++++
>>> 2 files changed, 55 insertions(+)
>>>=20
>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>> index 813cdb67b370..eac7212c9218 100644
>>> --- a/fs/nfsd/nfs4state.c
>>> +++ b/fs/nfsd/nfs4state.c
>>> @@ -2859,6 +2859,7 @@ static int
>>> nfsd4_cb_recall_any_done(struct nfsd4_callback *cb,
>>> 				struct rpc_task *task)
>>> {
>>> +	trace_nfsd_cb_recall_any_done(cb, task);
>>> 	switch (task->tk_status) {
>>> 	case -NFS4ERR_DELAY:
>>> 		rpc_delay(task, 2 * HZ);
>>> @@ -6242,6 +6243,7 @@ deleg_reaper(struct work_struct *deleg_work)
>>> 		clp->cl_ra->ra_keep =3D 0;
>>> 		clp->cl_ra->ra_bmval[0] =3D BIT(RCA4_TYPE_MASK_RDATA_DLG) |
>>> 						BIT(RCA4_TYPE_MASK_WDATA_DLG);
>>> +		trace_nfsd_cb_recall_any(clp->cl_ra);
>>> 		nfsd4_run_cb(&clp->cl_ra->ra_cb);
>>> 	}
>>>=20
>>> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
>>> index 06a96e955bd0..efc69c96bcbd 100644
>>> --- a/fs/nfsd/trace.h
>>> +++ b/fs/nfsd/trace.h
>>> @@ -9,9 +9,11 @@
>>> #define _NFSD_TRACE_H
>>>=20
>>> #include <linux/tracepoint.h>
>>> +#include <linux/sunrpc/xprt.h>
>>>=20
>>> #include "export.h"
>>> #include "nfsfh.h"
>>> +#include "xdr4.h"
>>>=20
>>> #define NFSD_TRACE_PROC_RES_FIELDS \
>>> 		__field(unsigned int, netns_ino) \
>>> @@ -1510,6 +1512,57 @@ DEFINE_NFSD_CB_DONE_EVENT(nfsd_cb_notify_lock_do=
ne);
>>> DEFINE_NFSD_CB_DONE_EVENT(nfsd_cb_layout_done);
>>> DEFINE_NFSD_CB_DONE_EVENT(nfsd_cb_offload_done);
>>>=20
>>> +TRACE_EVENT(nfsd_cb_recall_any,
>>> +	TP_PROTO(
>>> +		const struct nfsd4_cb_recall_any *ra
>>> +	),
>>> +	TP_ARGS(ra),
>>> +	TP_STRUCT__entry(
>>> +		__field(u32, cl_boot)
>>> +		__field(u32, cl_id)
>>> +		__field(u32, ra_keep)
>>> +		__field(u32, ra_bmval)
>>> +		__array(unsigned char, addr, sizeof(struct sockaddr_in6))
>>> +	),
>>> +	TP_fast_assign(
>>> +		__entry->cl_boot =3D ra->ra_cb.cb_clp->cl_clientid.cl_boot;
>>> +		__entry->cl_id =3D ra->ra_cb.cb_clp->cl_clientid.cl_id;
>>> +		__entry->ra_keep =3D ra->ra_keep;
>>> +		__entry->ra_bmval =3D ra->ra_bmval[0];
>>> +		memcpy(__entry->addr, &ra->ra_cb.cb_clp->cl_addr,
>>> +			sizeof(struct sockaddr_in6));
>>> +	),
>>> +	TP_printk("client %08x:%08x addr=3D%pISpc ra_keep=3D%d ra_bmval=3D0x%=
x",
>>> +		__entry->cl_boot, __entry->cl_id,
>>> +		__entry->addr, __entry->ra_keep, __entry->ra_bmval
>>> +	)
>>> +);
>> This one should go earlier in the file, after "TRACE_EVENT(nfsd_cb_offlo=
ad,"
>>=20
>> And let's use __sockaddr() and friends like the other nfsd_cb_ tracepoin=
ts.
>>=20
>>=20
>>> +
>>> +TRACE_EVENT(nfsd_cb_recall_any_done,
>>> +	TP_PROTO(
>>> +		const struct nfsd4_callback *cb,
>>> +		const struct rpc_task *task
>>> +	),
>>> +	TP_ARGS(cb, task),
>>> +	TP_STRUCT__entry(
>>> +		__field(u32, cl_boot)
>>> +		__field(u32, cl_id)
>>> +		__field(int, status)
>>> +		__array(unsigned char, addr, sizeof(struct sockaddr_in6))
>>> +	),
>>> +	TP_fast_assign(
>>> +		__entry->status =3D task->tk_status;
>>> +		__entry->cl_boot =3D cb->cb_clp->cl_clientid.cl_boot;
>>> +		__entry->cl_id =3D cb->cb_clp->cl_clientid.cl_id;
>>> +		memcpy(__entry->addr, &cb->cb_clp->cl_addr,
>>> +			sizeof(struct sockaddr_in6));
>>> +	),
>>> +	TP_printk("client %08x:%08x addr=3D%pISpc status=3D%d",
>>> +		__entry->cl_boot, __entry->cl_id,
>>> +		__entry->addr, __entry->status
>>> +	)
>>> +);
>> I'd like you to change this to
>>=20
>> DEFINE_NFSD_CB_DONE_EVENT(nfsd_cb_recall_any_done);
>=20
> TP_PRORO of DEFINE_NFSD_CB_DONE_EVENT requires a stateid_t which
> CB_RECALL_ANY does not have. Can we can create a dummy stateid_t
> for nfsd_cb_recall_any_done?

Ah, I didn't remember that a state ID was recorded. OK, then a
separate TRACE_EVENT is appropriate for nfsd_cb_recall_any_done.


> Note that nfsd_cb_done_class does not print the server IP address
> which is more useful for tracing. Should I modify nfsd_cb_recall_any_done
> class to print the IP address from rpc_xprt?

The IP address is recorded by the Call side tracepoints. I'm OK
leaving the IP address out of the Reply side tracepoints.


> -Dai
>=20
>>=20
>>=20
>>> +
>>> #endif /* _NFSD_TRACE_H */
>>>=20
>>> #undef TRACE_INCLUDE_PATH
>>> --=20
>>> 2.9.5
>>>=20
>> --
>> Chuck Lever

--
Chuck Lever