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:     Jeff Layton <jlayton@kernel.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 1/3] nfsd: add new tracepoint in nfsd_open_break_lease
Thread-Topic: [PATCH 1/3] nfsd: add new tracepoint in nfsd_open_break_lease
Thread-Index: AQHYo2rhSxHopoKUnUe/ulYoXZXJ/62VmDsA
Date:   Fri, 29 Jul 2022 17:20:45 +0000
Message-ID: <891E1B29-8E7F-4335-B3E2-492F12A090E2@oracle.com>
References: <20220729164715.75702-1-jlayton@kernel.org>
In-Reply-To: <20220729164715.75702-1-jlayton@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?EM0TNKJNMvUlK1x6qhV/XYU5TYbDysLt8YtAZFWfHR6sPbMpyyZzEflYqez+?=
 =?us-ascii?Q?rsteEvqoSOnc7FYl0AEKsuqHiSldQEo/FW2jt7jOl8I18eMcDfGKhjavylX9?=
 =?us-ascii?Q?SCGgtG/Z4fR9ZvdBJz+GsFbsF/ufFw39F1w0UEfMoup9defcRFMQVr64sARD?=
 =?us-ascii?Q?h1CuE0+ZC9CD0Rhzc3Vqh3xePsmzpbf9GJMCOuf4phBGFfEI+ze+UmCkmDwm?=
 =?us-ascii?Q?aJa06Zp4TM47IyR5qF0HWVeVeMeub/OzJLQ/n53K9SqxD19IJ8I3/utggJMV?=
 =?us-ascii?Q?+wYcJNCPBKbgUosLIkTNwhRtFdnhJGVZvstGh5uDca12kkZPNzn0T0rwjsRM?=
 =?us-ascii?Q?ZHkmRPRWk04WcitzYMipVEhRpCg2nAgQiZkM0m3Ma6EocOZbrIKBHbbfQZvU?=
 =?us-ascii?Q?b7Zhn5rCgp2lrj4jp6RuVOFpyOYLXodXXhciehubQbVkxjwGgxbpssbYcUII?=
 =?us-ascii?Q?ryMke8rpQLhiW1itUyHDKCNhyQwECBK/X/x41VXC/tKxUg4JYFa+/aUPvBqi?=
 =?us-ascii?Q?Jeu2phgFXjBeQvCQgtY/lWJ2iqlhd62puWCw2pgkrihTlqkudrtASLUbPH4n?=
 =?us-ascii?Q?mFBqQU0lnlXm5hsiTUDo83p3X7+IjzmR+xKXdW4ZWbr/pWTmPIEA3TytgDR0?=
 =?us-ascii?Q?8ZWtwtcYgHcHERJf4j6L/4c5gOCiVoikLvSdbl8gUz77wEf8QREHuyBhRi+E?=
 =?us-ascii?Q?eKeZSkM+5DFTCEQ5ShJMcPyJvO4t2Mxu2gp/ZgQFOhDgUdYMu8dFngrfCLat?=
 =?us-ascii?Q?Rc8BpaENQSk8Dv/zxrbD1+b/QI/Y+FQqJv0JWQmrn4gu5pul/bD7M8fzXEVz?=
 =?us-ascii?Q?7QfYB9JeM38EKml6wEj7OlzoGSlnqQTIL803qbj53iXADPGvRdiJNuS6dBG5?=
 =?us-ascii?Q?bOvR63p7RzJjTzDF79WcS0AWta3B7icEqLoxgvVJ0ArH5u9Fcd/Gm7aH65QX?=
 =?us-ascii?Q?Gl3ZGpRJOlrFMa5jL1FyRHkbsba8hJSQ05fjGl0X3khVJseRxzNXEE18bllE?=
 =?us-ascii?Q?vEwzC22jwNZ2yT9CrqJIDCEaC399rqHDU5mZZzCeb3EPY8fWiQroAFawT9GW?=
 =?us-ascii?Q?080TrzSuV7xAgrHTQJtzkiuYJdyPk9N71SWflqkSz3jQk9JZ+al5epMg6Be0?=
 =?us-ascii?Q?aHEn2/mP5ZzFFAxzcrJ1n8OXMS8vJqoLl5sPZwouwSVG9wKX6mOYMM44eSX4?=
 =?us-ascii?Q?ep1bYOu0C6R1/cdJqic5wGn9JFrEys+DuyPY8SAa3HEAEgcmh29flFotYJ4w?=
 =?us-ascii?Q?+kbocjAZ2Snzb6bCg2uV/G0gFPmnV0gzsJNAfW7N77U6gYwqHSH3GY5/xE6s?=
 =?us-ascii?Q?KzSI3WzfFo5RynvthFsn6NcvSmIqaYJjYBgbuu+plwKDk2F4ioQNP2yj/7Z3?=
 =?us-ascii?Q?JydwH4evjTUj43vbbgsRbMIsv6lJI1uuQlGaQTWe6MPhD3rdhwt1nx3yQMHD?=
 =?us-ascii?Q?Thg75EFFr3DO7MpRRXL4y21vqduMNhPhfUmTdOtu0yn12zKqL8xX1Adg3sPS?=
 =?us-ascii?Q?Er4svLwmhEixKe21Jw9n3dFkoEKV0sUmlF2zpg8ZS3LIeK5rMoyn+r5gnhdH?=
 =?us-ascii?Q?LyY2R+FUTfEwrtThO3EY5j4aTOI+JWW4yAxyFxwUA1o5EWPm/s0don2cP+QZ?=
 =?us-ascii?Q?6A=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <68B845C1EA202245BD023B5D15757455@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
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: 7b75040d-00c7-4207-5fb0-08da7186ac3e
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Jul 2022 17:20:45.5046
 (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: v9WZ1kDlTTZr1DzJFvpKcqhlAdulAAJgnx1pxVb2iycWjIADQR8YUs2gjidt5z7SarEDtuC1GQxGlefKlf+gLg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR10MB1313
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-07-29_18,2022-07-28_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 phishscore=0 bulkscore=0
 adultscore=0 mlxscore=0 mlxlogscore=999 spamscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207290075
X-Proofpoint-ORIG-GUID: KM9YvzeQsI-c8HpzAsbSVNZcBbm0t3ui
X-Proofpoint-GUID: KM9YvzeQsI-c8HpzAsbSVNZcBbm0t3ui
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 Jul 29, 2022, at 12:47 PM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/nfsd/trace.h | 16 ++++++++++++++++
> fs/nfsd/vfs.c   |  2 ++
> 2 files changed, 18 insertions(+)
>=20
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index 96bb6629541e..38fb1ca31010 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -531,6 +531,22 @@ DEFINE_STATEID_EVENT(open);
> DEFINE_STATEID_EVENT(deleg_read);
> DEFINE_STATEID_EVENT(deleg_recall);
>=20
> +TRACE_EVENT(nfsd_open_break_lease,
> +	TP_PROTO(struct inode *inode,
> +		 int access),
> +	TP_ARGS(inode, access),
> +	TP_STRUCT__entry(
> +		__field(void *, inode)
> +		__field(int, access)

trace_print_symbols_seq() takes an unsigned long,
so I prefer to store values that are passed to it in
an unsigned long field to make the type conversion
more obvious to readers.


> +	),
> +	TP_fast_assign(
> +		__entry->inode =3D inode;
> +		__entry->access =3D access;
> +	),
> +	TP_printk("inode=3D%p access=3D%s", __entry->inode,
> +		  show_nfsd_may_flags(__entry->access))
> +)
> +
> DECLARE_EVENT_CLASS(nfsd_stateseqid_class,
> 	TP_PROTO(u32 seqid, const stateid_t *stp),
> 	TP_ARGS(seqid, stp),
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index d79db56475d4..0edfe6ff7d22 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -729,6 +729,8 @@ int nfsd_open_break_lease(struct inode *inode, int ac=
cess)
> {
> 	unsigned int mode;
>=20
> +	trace_nfsd_open_break_lease(inode, access);
> +

IMO this tracepoint should include the incoming XID as well.
I checked: each caller of nfsd_open_break_lease() has an
svc_rqst pointer to pass in here, and that can be passed
along to the tracepoint.

Thus if we're going to change the synopsis of
nfsd_open_break_lease() perhaps it would be better to return
a __be32 nfserr status, since every one of its call sites
appears to convert the returned hosterr value to an nfserr.


> 	if (access & NFSD_MAY_NOT_BREAK_LEASE)
> 		return 0;
> 	mode =3D (access & NFSD_MAY_WRITE) ? O_WRONLY : O_RDONLY;
> --=20
> 2.37.1
>=20

--
Chuck Lever