Received-SPF: pass (google.com: domain of linux-nfs+bounces-1277-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <324d4e4e-4685-4341-a7b7-c73de473c57e@oracle.com>
Date: Mon, 22 Jan 2024 15:07:16 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [External] : Re: [PATCH 1/1] NFSv4.1: Assign retries to
 timeout.to_retries instead of timeout.to_initval
Content-Language: en-US
To: Benjamin Coddington <bcodding@redhat.com>,
        Chuck Lever <chuck.lever@oracle.com>
Cc: linux-nfs@vger.kernel.org
References: <20240122172353.2859254-1-samasth.norway.ananda@oracle.com>
 <995F0BB9-C709-4C3A-92F3-5EB9710A47F5@redhat.com>
 <bdd760d9-9900-43ef-8000-0e9ce0aa8b3e@oracle.com>
 <8B835117-498D-484E-B1DE-912FBBBCC74E@redhat.com>
 <Za7x2aYNRqc3nIbl@tissot.1015granger.net>
 <8DCCEB9D-5504-4DD4-A042-61C99D22A462@redhat.com>
From: samasth.norway.ananda@oracle.com
In-Reply-To: <8DCCEB9D-5504-4DD4-A042-61C99D22A462@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?NUduQUdSOHk4cUJzMmVjcTU3ZlZUTloxakp1VWVMUFVON1p5cFVoVVBOZUZo?=
 =?utf-8?B?VFYxSU80Tld4a3ZGQWN4TDg2VDFUVlo2N3Z2cTB3ejVYTHc0aVh2Mk1MSWRs?=
 =?utf-8?B?UkozZGlWVncxRHlHRjNBU1dPWFBaekpKNlltRVlJaFdCRTFzUXFLd3FrcjEy?=
 =?utf-8?B?bzVtVUt5N1FKT2V0TXA4R0YwK3Rhc3BrVXVxR0Q1MFJhczdRL29KTStTOXZV?=
 =?utf-8?B?N3VGcXk2aXFkVFFjMkdpY3puVStoUDlZazVoYStaY2dNa0NYbXk1VGhqMzRJ?=
 =?utf-8?B?L1l0N1VSTHV1S05KOFJ3QlNjNXZrSVg4NXBxdjV5NExQTnFZeGJLZHR5c0sz?=
 =?utf-8?B?NjZKRTR3NFJ5L1hEQk1ndkg0UEJiQlYrZWZqck5qZVRpZWg3V3R0eklHdDhC?=
 =?utf-8?B?WEFjWnNIYm5pSGIrYk03YWJSNjN0ZEhOeFBKaHF0MjVNc0NZd1FZTnZqNGdt?=
 =?utf-8?B?eWRJUXduR204ZlcwRFRualY4dkVxMVVPdERLZlhWWGJSeDZYS3MzVTdRTFhC?=
 =?utf-8?B?MXBKWHhjTkhYK3M1NzhTWjJJaDBOMDltMWtPa1lrTFc0R3psdFBVNXVFNWVU?=
 =?utf-8?B?Sll3d2VlYlJVbjc5NXRrRDI2UXRTeUFva0xaVklQb2o1VVRkZzliS2xXY2Ju?=
 =?utf-8?B?OG9kZWcra0krYjVDZzQwMVF5alJsbmpVbjJCdXFMcDBCeElKOWRBTkxqL1Zq?=
 =?utf-8?B?RWcyNldpS0F1TXozaXp2SWN4R3BpeG14elJqU0x4RnFDelpVK1FsUHhoTUNW?=
 =?utf-8?B?TjhxejlTYlRGajlQM1IvZktxR2g1WllBc3pLWUdya2c3ajFCZlVtS0NlVVNy?=
 =?utf-8?B?MElMZFhCUTVhd2xZNy9ZUFFVU3ZWSk9tRXdCbFNoazUxbXhOV2p4TkdqZ292?=
 =?utf-8?B?NWpEaWhhS3Mwajhid0pmVkZiTS8reFk3dUpPc1N0YlFZOUgrZ2hOS0ZUUkRL?=
 =?utf-8?B?dUluQ2pOWVNUdkt3eWoxT3RrRXBLNkF5eDRlQ2JXTzdnNVFFK0pSUGxzQjUy?=
 =?utf-8?B?VlFUZWk4Q2ZEUXJlUHliRk1PM1J5SGdEZWdSaUFPSVRIQmVZQUdOSHFZOVpz?=
 =?utf-8?B?Q2tDclo5NUNUd2l0aHRORnVoTm9xTVI1UnNVRW1uSnFuYVV6Q3BjNVg2MXFk?=
 =?utf-8?B?SXFzWXhPZWUyUFI4ckNxTnlwQWlUSTNXSXZ1eUtVZ0tIOU5BYU9TY1dEcmFI?=
 =?utf-8?B?dGZJM1BCVDVmTURoNldNeER3bnB6b0Z0V01VWXN3Y29QVVdzQXdvenFNOVAz?=
 =?utf-8?B?Z3ZiUEluSm42dWdzRUZCMTRiVjZqR0JVUFA3YS9JTktIRTBvaTRJNzVOZmpO?=
 =?utf-8?B?ZkY3WUM4US9NK2Q2Y3VqTGpXU1hIL3k2R2Mvb1hwQjRob2R1aHducjNxT2M5?=
 =?utf-8?B?aStQZUxodzFCK2pGREhTeW5GN2pXRFRYcGpFNmxtNHRCNEJBRUwzaVBLRVVO?=
 =?utf-8?B?OHZGdE8yd1R5VGxWRVBlZkRmZUVJWHdZbnV6c0ZzRTIzL25JbVJzRUpjVTMv?=
 =?utf-8?B?Um1UazFzU05LTWg1UWJMV2pUZXQrLzlaUFNTaXJabisyWTBvVTNYRWUyRDNG?=
 =?utf-8?B?SllkRzRmQU8vb2ZCVlNpdE5KN2N6UFlycjVPRjRPOW80b0JjZ2REL1ZPUTQw?=
 =?utf-8?B?N3JrRTZ6QW9LYXpqM0FSbC9zcEk3Y0huOHJLc1lrbWlraDQ0enVkNk1NNTlm?=
 =?utf-8?B?SmJIc1IzQjZ0UGlseHZ1U3pkSUoyaCtLWUg3M3ovbXdXOCt1RlR5bXNwdnpF?=
 =?utf-8?B?a2V5SWU2YlRnMVk2ZUlnbGhhaG51QlMxSlIwMWJ0WGRtSzdBTi8rc3I3UkRR?=
 =?utf-8?B?NUFDVTAyMGs1cWFBcEhkZTdYL2hyREFlM3h1QjJNR25hZVliUzFsN3IzQklE?=
 =?utf-8?B?VFJESmFLdXNMekNVSk01dkdDWisvbDVIejM3cTh3Q3huNFoweDBNQTZsSUVl?=
 =?utf-8?B?Z0hoY09adi96WHNzbVlHTFlmbW9xbEpycWJEVkxKZFIwdDgwcG1NM1lNUjRI?=
 =?utf-8?B?RzJFSW52ZE91WmxIazREZnpwaHZpaGR6c2RqTWxkVUdZQ3ovYWM2MTZualJp?=
 =?utf-8?B?TE1qZWhhTHpGcHpjaHQyYlUxQ1JuNnR0UWpmTDJxSDNySEQvR094cjB4Nk5G?=
 =?utf-8?B?bVpGcUs2a2kvV3ArT1hMQ2lwNFhxMHJrOGlxeEhJSmszaFJDMW9MWnNZdDVw?=
 =?utf-8?Q?NdGQaJFCVXKON1FKnFPxrmA=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	nz0TkJPJ3pW6XqpVvvjwutnYL7OmCSBYhdsNC7nIO0saBLSeuqZ/fAGKRDlhk10h2CouGKN0YFcz7xjcG/A79wjHMXcvG9PCVvMjz5Dq+cgfKfrtpcFtR+zg3C8o9CKdzMYI8OpVDN0fCPm/GJZaLoU6MbRtJw8iUSn7AV8qEpEK8Vf4ud//53UF/LNd1Lnj5c1VjAs00LX6xhJtwtmEr7CoicLL+tHidC0ETNzusmMX37N/3//amayiCVpn0dMeJTeUPhKWaf0ORAmC3tABlHtlA+oymi5LiFxH/+AdjtnjAxgTWTjTLdn5RpSWG8yqkDVk/LHOqlj7N8VeZoieGaT3hHMVf4cCpPpTUnaj7Zrn0/5K81HYaeoZRMKUG6JnVqgY+rhQVrCMyvJUZP7mxTB8YLVIYSj+2+IDW3C0ag5vQt3e4AS6zRLvVXlnXGMtK8rnDV5T1t6S4AYGSwfCb1qmec7jbc6nZBwB0fr32nkhLLvhyBywyJS6+iKxrfMZlzU/8VX1Y5XBlP2m3rg5UumpaSpavGVtKsXNKMGsZo59eT8sCi6tU1Kz5e2RHXf+catMBhiIakIAOrqHPfVZhV22TObVofT+0KnaVZF5b0g=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 18b30d2d-3a4f-4cf4-f406-08dc1b9ee222
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB3817.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jan 2024 23:07:19.3758
 (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: s8NCEGK1XTcdaIThH7RQzThc2A1DYiVrRDQdls13HaCs3Ls1ssLDGLLXltI5PLa+S3YAONtiWU/TYhxQ1zusD098/p/EorKLM8XNtjgDe5tGHGJ2ac41zq2apR5GC0b1
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN6PR10MB7518
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-01-22_12,2024-01-22_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 suspectscore=0
 phishscore=0 malwarescore=0 mlxscore=0 mlxlogscore=999 spamscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2401220165
X-Proofpoint-GUID: HiMUbTCSgbmznv9Lcu4sBIyvhwPUu4X-
X-Proofpoint-ORIG-GUID: HiMUbTCSgbmznv9Lcu4sBIyvhwPUu4X-


On 1/22/24 3:03 PM, Benjamin Coddington wrote:
> On 22 Jan 2024, at 17:53, Chuck Lever wrote:
> 
>> On Mon, Jan 22, 2024 at 05:46:56PM -0500, Benjamin Coddington wrote:
>>> On 22 Jan 2024, at 17:44, samasth.norway.ananda@oracle.com wrote:
>>>
>>>> On 1/22/24 2:41 PM, Benjamin Coddington wrote:
>>>>> On 22 Jan 2024, at 12:23, Samasth Norway Ananda wrote:
>>>>>
>>>>>> In the else block we are assigning the req->rq_xprt->timeout->to_retries
>>>>>> value to timeout.to_initval, whereas it should have been assigned to
>>>>>> timeout.to_retries instead.
>>>>>>
>>>>>> Fixes: 57331a59ac0d (“NFSv4.1: Use the nfs_client's rpc timeouts for backchannel")
>>>>>> Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
>>>>>> ---
>>>>>> Hi,
>>>>>>
>>>>>> I came across the patch 57331a59ac0d (“NFSv4.1: Use the nfs_client's rpc
>>>>>> timeouts for backchannel") which assigns value to same variable in the
>>>>>> else block.  Can I please get your input on the patch?
>>>>>
>>>>> Oh yes, this a good fix.  Usually the maintainers won't pick up a patch
>>>>> that's only sent to the list, rather the patch should be addressed to them
>>>>> directly and copied to the list.  Can you re-send this patch to:
>>>>>
>>>>> Trond Myklebust <trond.myklebust@hammerspace.com>,Anna Schumaker <anna@kernel.org>
>>>>>
>>>>> and copy linux-nfs@vger.kernel.org?  You can also add my:
>>>>>
>>>>> Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
>>>>
>>>> Sure, I will do that. Thanks for the review.
>>>
>>> Can you also fixup the block above the hunk you posted?  Its backwards there too!
>>
>> It's backwards in a different way.
> 
> Its an artifact of my text editing..
> 
>> And should you set to_maxval in both places as well?
> 
> IIRC it does not need to be set there.

Ah okay. So it should be like this right?

if (rqstp->bc_to_initval > 0) {
	timeout.to_initval = rqstp->bc_to_initval;
         timeout.to_retries = rqstp->bc_to_retries;
}
else {
         timeout.to_initval = req->rq_xprt->timeout->to_initval;
         timeout.to_retries = req->rq_xprt->timeout->to_retries;
}


Thanks,
Samasth.
> 
> Ben
>