[Intel-wired-lan] [PATCH net-next] ice: Fix incorrect timeout in ice_release_res()

[Ding Hui]

On 2025/12/6 17:46, Simon Horman wrote:
> On Sat, Dec 06, 2025 at 10:42:36AM +0800, Ding Hui wrote:
>> On 2025/12/6 5:09, Loktionov, Aleksandr wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Intel-wired-lan <intel-wired-lan-bounces at osuosl.org> On Behalf
>>>> Of Ding Hui
>>>> Sent: Friday, December 5, 2025 9:16 AM
>>>> To: Nguyen, Anthony L <anthony.l.nguyen at intel.com>; Kitszel,
>>>> Przemyslaw <przemyslaw.kitszel at intel.com>; andrew+netdev at lunn.ch;
>>>> davem at davemloft.net; edumazet at google.com; kuba at kernel.org;
>>>> pabeni at redhat.com; Keller, Jacob E <jacob.e.keller at intel.com>; intel-
>>>> wired-lan at lists.osuosl.org
>>>> Cc: netdev at vger.kernel.org; linux-kernel at vger.kernel.org; Ding, Hui
>>>> <dinghui at sangfor.com.cn>
>>>> Subject: [Intel-wired-lan] [PATCH net-next] ice: Fix incorrect timeout
>>>> in ice_release_res()
>>>>
>>>> The commit 5f6df173f92e ("ice: implement and use rd32_poll_timeout for
>>>> ice_sq_done timeout") converted ICE_CTL_Q_SQ_CMD_TIMEOUT from jiffies
>>>> to microseconds.
>>>>
>>>> But the ice_release_res() function was missed, and its logic still
>>>> treats ICE_CTL_Q_SQ_CMD_TIMEOUT as a jiffies value.
>>>>
>>>> So correct the issue by usecs_to_jiffies().
>>>>
>>>
>>> Please add a brief "how verified" paragraph (platform + steps).
>>> This is a unit-conversion fix in a timeout path; a short test description helps reviewers and stable backports validate the change.
>>>
>> Sorry for not being able to provide the verification information, as
>> I haven't actually encountered this issue.
>>
>> The ice_release_res() is almost always invoked during downloading DDP
>> when modprobe ice.
>>
>> IMO, it seems like that only when the NIC hardware or firmware enters
>> a bad state causing single command to fail or timeout (1 second), and
>> then here do the retry logic (10 senconds).
>>
>> So it's hard to validate on healthy NIC, maybe inject faults in low level
>> function, such as ice_sq_send_cmd().
> 
> In that case I would suggest adding something like this:
> 
> Found by inspection (or static analysis, or a specific tool if publicly
> available, ...).
> Compile tested only.
> 

Sure, I'll send v2 later.

>>
>>> And you can add my:
>>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov at intel.com>
>>>
>>>
>>>> Fixes: 5f6df173f92e ("ice: implement and use rd32_poll_timeout for
>>>> ice_sq_done timeout")
>>>> Signed-off-by: Ding Hui <dinghui at sangfor.com.cn>
>>>> ---
>>>>    drivers/net/ethernet/intel/ice/ice_common.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
>>>> b/drivers/net/ethernet/intel/ice/ice_common.c
>>>> index 6fb0c1e8ae7c..5005c299deb1 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice_common.c
>>>> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
>>>> @@ -1885,7 +1885,7 @@ void ice_release_res(struct ice_hw *hw, enum
>>>> ice_aq_res_ids res)
>>>>    	/* there are some rare cases when trying to release the
>>>> resource
>>>>    	 * results in an admin queue timeout, so handle them correctly
>>>>    	 */
>>>> -	timeout = jiffies + 10 * ICE_CTL_Q_SQ_CMD_TIMEOUT;
>>>> +	timeout = jiffies + 10 *
>>>> usecs_to_jiffies(ICE_CTL_Q_SQ_CMD_TIMEOUT);
>>>>    	do {
>>>>    		status = ice_aq_release_res(hw, res, 0, NULL);
>>>>    		if (status != -EIO)
>>>> --
>>>> 2.17.1
>>>
>>>
>>>
>>
>> -- 
>> Thanks,
>> - Ding Hui
>>
>>
> 
> 

-- 
Thanks,
-dinghui