Received-SPF: pass (google.com: domain of linux-kernel+bounces-97712-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <1bb40d6c-a542-437d-8e81-9ae834bc079a@oracle.com>
Date: Fri, 8 Mar 2024 16:55:13 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] RDMA/cm: add timeout to cm_destroy_id wait
Content-Language: en-US
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: dledford@redhat.com, linux-rdma@vger.kernel.org,
        linux-kernel@vger.kernel.org, rama.nichanamatlu@oracle.com
References: <20240308005553.440065-1-manjunath.b.patil@oracle.com>
 <20240308141852.GR9225@ziepe.ca>
From: Manjunath Patil <manjunath.b.patil@oracle.com>
In-Reply-To: <20240308141852.GR9225@ziepe.ca>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?NTNMTWk2c2VDSmdwa3daUWFVaE1Uckg3ZzhrRStIZDhGR2NQL0kyTlVYblNI?=
 =?utf-8?B?NXJBUDFEZmt5ZExlcG5uTSsxRmQyb1hoZ1RXcmVFTzdMT3luTFQycWkxTm9O?=
 =?utf-8?B?UzJuMGJXNmRtWWk3T0dzQllCUTZjb25YYmh3ODdERkNzYnQzWWtuNkZRblhv?=
 =?utf-8?B?N29nUXlsc01HYTlEeXhZOFhXZEVkQnBTYVVRM0xGT3BSVmhQZHZYSDNUWUp6?=
 =?utf-8?B?eFd3WWJKempGM1RWWVFTOUk0eUdhSHJuTnZkWXVONWdJRzMzWFR2WnRNOG45?=
 =?utf-8?B?RXc3bm9tS2ZhTjRXaUFaMFJYRlhwU1kyVzhKQlB2U1R0c0ZzeGlDM2wvdGxV?=
 =?utf-8?B?cE9Jc2Vzb24xRTZVaFo2bGUvaGVJZjM2aFBSSERlSjZMcktyUExKZHhQYUtU?=
 =?utf-8?B?WlNRY1NCbkJCSmVBZXJBMm43TklrZ1lpTUN1YVlTWDB1Z3dXclVkdWxNR3pU?=
 =?utf-8?B?NEx6UmUvNk5UT0l5ekhicUxpSVpHdEpHdXBidVdUQitvaTNPOGpGV3NLUTlG?=
 =?utf-8?B?QzdSbTYrSFFLRlhkUExjc3U1MHZnWmFoYUZDeWdwRjVuS2l4TWZsU0t0dFhp?=
 =?utf-8?B?WldMdjZXOGdrRm1WSjZWNUlXOUZtbGlJU2RZVzlFdHNZWFphUjErNHBhL1Iy?=
 =?utf-8?B?VUlEY0ZjYlRZVHVXM2NwVGhMU0tldU9SaWtKck1oc1J0Vzl4T2FVYkZGZDU0?=
 =?utf-8?B?WlFYVGNZSW5sa3NmMnh6UXVtUjJ5WGNYS3VpYk5uRXJOUlJSemhqNlJpU0JQ?=
 =?utf-8?B?eC9jNXFZVyt4dkJBYVh6Wk9zU0hZUkVlYTNSQ2IyRGdMRkVZL1B3OTIxSHNz?=
 =?utf-8?B?ckE3WStvbm05RVUwWHVBODUxYllVT2xzd2JPaDB5aGk3T1JqbUx5VHFOS0VH?=
 =?utf-8?B?ZXBpV0hiS3UxMFVDay8vRjA3WDdocEtyMVl6SlVwYW9xLzlic2FnZUdreHlC?=
 =?utf-8?B?aCtLMXVBQXp2cnp2cmlHaElaRGJkNkJkRWpmdTh0MW1RdmlIOWUxckVhM0xw?=
 =?utf-8?B?WFFIZmxsKzRzOHMxYk13T0pGMUl5YlV4VVdMWTJPWlJqdFNIVitFVUZSSHgr?=
 =?utf-8?B?aS9WWWRTKzR0QjBnSFVTYzlxbFl4NzhGL2VlZk1rcUVFTzh6ejJzOUFPVHdX?=
 =?utf-8?B?ZURMQ1N5cUVINFhsUkdiNDhvakJoWXp1VmNrU1grSFdUUFlCNCt4RTEvMWI1?=
 =?utf-8?B?V2NLb3R6K1ZvTmNHRXgvQVlGRkM5cEplaUdjSlc0QXVUT2xqdzU0NHE3ZThZ?=
 =?utf-8?B?OVQ1dUZxRFdIanh0aVl0S2c0RlRuZzRTVVNvMllJa0FSNjkvNEVvMTA5ZXBm?=
 =?utf-8?B?M3ZQNk5kZXpleEFMU0I2bTlBTFFta1dPcldXSk9xenRsZzRheG9CdnNVYXR1?=
 =?utf-8?B?ZGZYSDhZSVI0WWdoVm5rTU50S1kzR2hob2tiV3ZPd2dTTjlMd1FPNTBlaGY2?=
 =?utf-8?B?WHhidTBkWnppcndCNkhlRktyMVJSTkRnT2ZGcWVOVk9Uc0dJaDkxaFI5OThO?=
 =?utf-8?B?ZWZEZ2h4bUZhREFrSTRkWFV1RVFCRE4zbzY1MGVuTjkrOVAyVTJKYVc2aGd6?=
 =?utf-8?B?UkQwVEhyQ0tTMDlMaFA2VnIzYlBtLzFGbWJjOVZqZ0xrS1M5N2FTNUttY1Bp?=
 =?utf-8?B?WXRnSGh2QSs1MmIrUE01dTRlcmRTYis0Y1FWRXF6blRTTlhiRWYyZGszZHRk?=
 =?utf-8?B?MXBXUnFHRlVIZnNSK25zdkNYdDBxWjhZKzZnSjdNcHg0VGFuYXpaVXNJQk9Q?=
 =?utf-8?B?cTFla3JKTGFsZGY2MEF3WUxCM3lveVVOSHBDSTJtNTAxdkloWllFYmNCQVBL?=
 =?utf-8?B?Z3RLN0xQeGdXYkIrOVZRb1RCZkV4Smowa0hmMjZ0QVplbE9VV3BVeVdIblUr?=
 =?utf-8?B?dEpPaWtacGV1U2NKRHVTWlJSd1MzNTZMdVpiaGdYWW5BMFcvalBXa0cvOGww?=
 =?utf-8?B?eXdQaC8rOW5seUd6eXE2TkpzN21FNHBYVlB0S3gxcWlxMHpaWlVuSVF5NFh4?=
 =?utf-8?B?b1Job2JWSFVKYlBobGNqYUxSSVB5bjJvSC9NL2tnSGI3a1RIeDRtNmJWTnZI?=
 =?utf-8?B?L08wcXptR05RRlM4OGV6dC9SYmI2NS9xRHk3eWJCbjZEVDZkMSswakljWmdp?=
 =?utf-8?B?R0RMMkNSSjdUVU1aaU9uZmRqak82NlAyMmVlSWc4ZFJ3VVU0cy95alJ0Nkt2?=
 =?utf-8?B?ZGc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	iGJRGWBeoFJni5HrOtfS1zjLMMZLRoN0f5plHPyMYW3sK0Wjx9dBTwxzroAXth1d4qafK12LI14f0vONeNho+2pvvCJx7E5gJ3pJdLr5t+mLvKh8Gxg6n9bztcY6egUKIe9WXlwApN3idwHDm9VSdTxU9fYhvBL/dJO+jKjKZCRPvMWKXgT06lnEIAhfPnd4m4G8QfELcGZ5MnVMmUxyGRUeNrQthV6C8s5Z1tQRnnDVstvW3qmMvgKP7GMIR12HRJLF/IYsFsK5rbF2Y+nw5pbuEGRkwGmWzPOy/VgNqUG3HxxP9WssXkaMwylgxiHYqcuIcQr2UWf/1FPExuezIhLqOjrLJM40H2zsT2mpRbIzRimzH5GdcHFd9tBLD4luyrzimNMc5/oNQIPuQ6oU4Wyf5N8NVzQ53XNMfnhjuJ7e5xGWoU4cspOX94rSLmR2VNXxb81in064Ow0RUPu0OnvrwMQE3RY8Bsqw3zZOBcv5mOVVfUsTO5bW2pyjS31o5dZyohWzKspiuk1GWAfDv9hNKJ3YrN9vZJQsoRjCCYexvrcrkXo9xplofNzn3UGM/2cRVgsWADkzRe15gcSYI4hNjiJH8ZZoOlgwh9IjwVw=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f8c82840-b523-4355-68e0-08dc3fd39630
X-MS-Exchange-CrossTenant-AuthSource: CY5PR10MB6216.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Mar 2024 00:55:17.1806
 (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: r1R2NDilWeF0w9leoZHOgOW6NfEWFkvZ89OANYaXGqpsSEIIGvItZUv+FT1lAylRjYlB9W+1WnsHP3Vlr3kSWn8t9ympAcWLSgcWAfpHA7w=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB7851
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-03-08_08,2024-03-06_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxlogscore=999
 suspectscore=0 adultscore=0 bulkscore=0 malwarescore=0 mlxscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2403090006
X-Proofpoint-ORIG-GUID: NA56oA6den0DXuCkD0pK8sP8vqTkdfMm
X-Proofpoint-GUID: NA56oA6den0DXuCkD0pK8sP8vqTkdfMm

On 3/8/24 6:18 AM, Jason Gunthorpe wrote:
> On Thu, Mar 07, 2024 at 04:55:53PM -0800, Manjunath Patil wrote:
>> Add timeout to cm_destroy_id, so that userspace can trigger any data
>> collection that would help in analyzing the cause of delay in destroying
>> the cm_id.
>>
>> New noinline function helps dtrace/ebpf programs to hook on to it.
>> Existing functionality isn't changed except triggering a probe-able new
>> function at every timeout interval.
>>
>> We have seen cases where CM messages stuck with MAD layer (either due to
>> software bug or faulty HCA), leading to cm_id getting stuck in the
>> following call stack. This patch helps in resolving such issues faster.
>>
>> kernel: ... INFO: task XXXX:56778 blocked for more than 120 seconds.
>> ...
>> 	Call Trace:
>> 	__schedule+0x2bc/0x895
>> 	schedule+0x36/0x7c
>> 	schedule_timeout+0x1f6/0x31f
>>   	? __slab_free+0x19c/0x2ba
>> 	wait_for_completion+0x12b/0x18a
>> 	? wake_up_q+0x80/0x73
>> 	cm_destroy_id+0x345/0x610 [ib_cm]
>> 	ib_destroy_cm_id+0x10/0x20 [ib_cm]
>> 	rdma_destroy_id+0xa8/0x300 [rdma_cm]
>> 	ucma_destroy_id+0x13e/0x190 [rdma_ucm]
>> 	ucma_write+0xe0/0x160 [rdma_ucm]
>> 	__vfs_write+0x3a/0x16d
>> 	vfs_write+0xb2/0x1a1
>> 	? syscall_trace_enter+0x1ce/0x2b8
>> 	SyS_write+0x5c/0xd3
>> 	do_syscall_64+0x79/0x1b9
>> 	entry_SYSCALL_64_after_hwframe+0x16d/0x0
>>
>> Orabug: 36280065
>>
>> Signed-off-by: Manjunath Patil <manjunath.b.patil@oracle.com>
>> ---
>>   drivers/infiniband/core/cm.c | 20 +++++++++++++++++++-
>>   1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
>> index ff58058aeadc..00a16b08c7e2 100644
>> --- a/drivers/infiniband/core/cm.c
>> +++ b/drivers/infiniband/core/cm.c
>> @@ -34,6 +34,7 @@ MODULE_AUTHOR("Sean Hefty");
>>   MODULE_DESCRIPTION("InfiniBand CM");
>>   MODULE_LICENSE("Dual BSD/GPL");
>>   
>> +static unsigned long cm_destroy_id_wait_timeout_sec = 10;
> 
> Don't need this to be a variable, just make it a #define
sure. I will send v3 with this change.
> 
>>   static const char * const ibcm_rej_reason_strs[] = {
>>   	[IB_CM_REJ_NO_QP]			= "no QP",
>>   	[IB_CM_REJ_NO_EEC]			= "no EEC",
>> @@ -1025,10 +1026,20 @@ static void cm_reset_to_idle(struct cm_id_private *cm_id_priv)
>>   	}
>>   }
>>   
>> +static noinline void cm_destroy_id_wait_timeout(struct ib_cm_id *cm_id)
>> +{
>> +	struct cm_id_private *cm_id_priv;
>> +
>> +	cm_id_priv = container_of(cm_id, struct cm_id_private, id);
>> +	pr_err("%s: cm_id=%p timed out. state=%d refcnt=%d\n", __func__,
>> +	       cm_id, cm_id->state, refcount_read(&cm_id_priv->refcount));
>> +}
> 
> WARN_ON? Is the backtrace valuable?
No. I would prefer just pr_err to avoid noise if there are multiple cm_ids are stuck due to any lower layer issue.

Thank you for your feedback.

-Manjunath
> 
> Jason