Received-SPF: pass (google.com: domain of linux-kernel+bounces-145370-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <258e2dba-da30-795f-6408-85c06e137b61@axis.com>
Date: Mon, 15 Apr 2024 16:35:02 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.0
Subject: Re: [PATCH v2 1/1] smb: client: Fix hang in smb2_reconnect
Content-Language: en-US
To: Steve French <smfrench@gmail.com>
Cc: linux-cifs@vger.kernel.org, samba-technical@lists.samba.org,
 sfrench@samba.org, pc@manguebit.com, ronniesahlberg@gmail.com,
 sprasad@microsoft.com, tom@talpey.com, linux-kernel@vger.kernel.org,
 rickard314.andersson@gmail.com, kernel@axis.com
References: <20240409112758.467112-1-rickaran@axis.com>
 <20240409112758.467112-2-rickaran@axis.com>
 <CAH2r5msm+xFyMPCJV7Luf6aQ04cHOhevmjJi8J-2Bf711T1ziQ@mail.gmail.com>
From: Rickard x Andersson <rickaran@axis.com>
In-Reply-To: <CAH2r5msm+xFyMPCJV7Luf6aQ04cHOhevmjJi8J-2Bf711T1ziQ@mail.gmail.com>
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?Z0pvcG5ZeUhHZXJCZkRzNVk2RUluZWdpdkJ4ak9pUFpQSVo3NzJxZFUzY25s?=
 =?utf-8?B?RE1wSTY4WmpjazFuRzRYNlRUTjlkRTh1dTZReTBySW1rZWFqSGdpT0ltaWVp?=
 =?utf-8?B?VlVQOEdwMXQzUTlIWWk0LzhlclFGZllVeE8zOUlYRmorZHA3bTRodm80TUF1?=
 =?utf-8?B?UnhGZmhxaVJwd3pRTjhyQW9lN0twUVREUFllNEpGSGR2eGswcW5ZOGRiUWNz?=
 =?utf-8?B?c1V1UW5nVWdiNUJpVGdQZm9pSW9OVSt3Z2dsb1F0VTN1QWtQVFlMTzh4dDdV?=
 =?utf-8?B?cm9FNG8yVHlWaTFoZmtNQ0V6VGNicEw0Q2plSGhhOUxXVUhhRlhMYVRxS0Ry?=
 =?utf-8?B?QnkzN21pUTlUM1YwOEpRV0M5OW1rb2tXY2NVQU9vNm42ZUxOVDlLdEw5N1BF?=
 =?utf-8?B?RUp0d0RQVGtmTE9LdXRMNVNEWVc2clFwRzlOREpoRmhHa3RqVWFkSDNHSTlj?=
 =?utf-8?B?a0VWRStNZ0p5dWhVYkxZY3Mvbm9hcWc2Z2tXci90SlJWWFZuMHZLbXNUc2N6?=
 =?utf-8?B?QmFNOUVCRFdQWjdqZUQwU2ZPalBIN0dUOVVSVktYUEFYR2dLalJ0Mmt3MFY3?=
 =?utf-8?B?b3RENWEyd2lhSXFVZG5udTNhamdzMEJVQXFuUTBIcTBlVTlFTTBBQzJtZkpz?=
 =?utf-8?B?WXR5MDVxUkF3c3RhUS9TWTNBaWNSRWp6KzlUVEtnNDJ6NUJ6czl5d2RvcDZD?=
 =?utf-8?B?TUZWdzN1SFFwNnk1VnpDY1VrUFFidWNTN1NlL3hvdHBOb2lLTEVvMkVneURY?=
 =?utf-8?B?RWZxMmdIa1dDTklqdlhjTHRsZ1MxRkgxL0JzTEhwZU9zVjduYlluQlBSNHQ4?=
 =?utf-8?B?S2FteVJxZkFyUlZzRUJiMUsrdE5qSXhqRXV5NnVFNVlJTXEvZmEyeTZnZG14?=
 =?utf-8?B?eG9qL1RYdWJuaVRnWHNuWFFIVWkyVm5FUU11YmllZ2tHbnNab1k1cEZCakJI?=
 =?utf-8?B?V0g5L3l0OCt3NXo1c01lSWQrcjhSbWJpYTU0bGxuMG81ZlVWaXBmWURDcWNH?=
 =?utf-8?B?L0VPMzFreE8rckhVUGF2MGtWMG15WW1FWGVsRVA4Q21aUnBzSzJpelhVRXUz?=
 =?utf-8?B?Z1htZW15Z3BIS2VjWXFuYjYwUlZHYUJuaElZcDRseWc5d1IyZm9xQkR3UEx4?=
 =?utf-8?B?WEF3ZWNYOUNwYUdoODVHdHhXR281QWg3eHprcGFRcEZHSWYwOWY4SGpIci85?=
 =?utf-8?B?ai9waU5yS1oyWWZvZ2s1VVNPa3hnZWwyc3pxb2p3aDM5N24wbno3VisyVGdp?=
 =?utf-8?B?R3pFNjdkYS9hMThWQjEzQ1FMOWhsVjhzS3lkNW8yTzNIUlJoYzNjUkQ5MjVU?=
 =?utf-8?B?SythcFQ0ajJkVWNZUGlzVHR5dlN6bUNqRTZQMUcrdFE1UkxxcWkyZGVLMHRR?=
 =?utf-8?B?a3JNNmhaVEk5aEV6Zm5yOXd5WkEvUkJSVGgwaWVWRjRUTmVhMzRsc3RXWVZ2?=
 =?utf-8?B?ZGdQRUd4RkowRk1XclFBcnp4ZHVDWkl6UTBUcXBMZVhYaGtzaVFuMlNURTM4?=
 =?utf-8?B?a2pqeFpZTDZldGVCUmNJajQ1a3Jyd05Gam5odlF2ZjJYRCttTmJ2SEJKOVFp?=
 =?utf-8?B?WWdjU1ZyTHI3UkxHbnpXVDA0MGVwaGFCK3NlRDZzbm1ZQ29oaGxjWENpaTRC?=
 =?utf-8?B?bDlURXpvZ1hEZHl6bUxpUUc2aDZ3cGJIM1lNUWl2dHNyY1hLK09wSysvLzJv?=
 =?utf-8?B?dUVUaFBGcit6Z0pqeWVhdVpPYTIxcTAyTU5RWWg0eldWNXRUcXlsOW5ydXMw?=
 =?utf-8?B?b0dUMFdldWhyZkRheFlnbXM3cm4ydGlvMmYvT3JlY2xLM3F0WTl5QnA0Mlor?=
 =?utf-8?B?UW1wU1BaSVo5eUlHSzNRcUpvSkE2T1FlNDVpMllaaVRkUUVrQVo4SEN1NGc2?=
 =?utf-8?B?QUhTS2JkZ2duYndyNjFJM0ZoQ3hvbzczaGpGY1JPZWpKbGN3anFiYXAyMlBn?=
 =?utf-8?B?NkFFR2JZa3dtN3lFNmg3dkE2bjM3dElGOU5qc2QzTzR3MzBuQWg2bld4bGlo?=
 =?utf-8?B?cUp1VWhSQ2p0dHFpSGZyeHdBaGhCS3ExMXN2bEh5NlptOGNQRGpEMmNvbGIz?=
 =?utf-8?B?cEk3Wmc4dEtBWXM5SjBWTGpVNDV1ejZkQWpuMGpuQWNTYVhDVHFESzU2WDNk?=
 =?utf-8?Q?pYUxG7slenJS+zjeSLfSX+K6a?=
X-OriginatorOrg: axis.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 74f03977-dd08-4fd6-40ad-08dc5d593cf0
X-MS-Exchange-CrossTenant-AuthSource: AS8PR02MB9505.eurprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Apr 2024 14:35:03.6912
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 78703d3c-b907-432f-b066-88f7af9ca3af
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: usXjwbChVDLb4sf1vEAYyOZbsfHgr7Fb+Hu9CNTYACo6bMqwMFQpkClR6OZnVGveiHWSI2NUJ6F/ZECZ9Zr/cg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR02MB9210

On 4/13/24 11:02, Steve French wrote:
> Shyam and I tried some experiments and there are two cases where the
> patch breaks:
> 1) ChangeNotify will time out
> 2) Certainly byte range lock calls (they can be allowed to block) will timeout
> 
> An obvious alternative would be to not make this change for the
> commands like ChangeNotify and blocking locks but allow it for the
> others.
> 

Would it make sense to make the patch less intrusive by doing something 
like the following?:

@@ -1209,7 +1216,12 @@ compound_send_recv(const unsigned int xid, struct 
cifs_ses *ses,
         spin_unlock(&ses->ses_lock);

         for (i = 0; i < num_rqst; i++) {
-               rc = wait_for_response(server, midQ[i]);
+               if (flags == CIFS_NEG_OP) {

+                       rc = wait_for_response(server, midQ[i], 50*1000);
+               }
+               else
+                       rc = wait_for_response(server, midQ[i], -1);
                 if (rc != 0)
                         break;


So, we are just waiting with timeout in the case it is a "CIFS_NEG_OP".

Note that I am not familiar at all with this code base.

Best regards,
Rickard