From: Simon Horman <horms@kernel.org>
To: Ding Hui <dinghui@sangfor.com.cn>
Cc: anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, jacob.e.keller@intel.com,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH net-next] ice: Fix incorrect timeout in ice_release_res()
Date: Fri, 5 Dec 2025 16:16:06 +0000 [thread overview]
Message-ID: <aTMFRkYZGtk3a_EP@horms.kernel.org> (raw)
In-Reply-To: <20251205081609.23091-1-dinghui@sangfor.com.cn>
On Fri, Dec 05, 2025 at 04:16:08PM +0800, Ding Hui wrote:
> 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().
>
> Fixes: 5f6df173f92e ("ice: implement and use rd32_poll_timeout for ice_sq_done timeout")
> Signed-off-by: Ding Hui <dinghui@sangfor.com.cn>
Thanks,
I agree with the analysis above and that the problem was introduced
by the cited commit.
As a fix for code present in net this should probably be targeted
at net (or iwl-net?) rather than net-next. But perhaps there is
no need to repost just to address that.
> ---
> 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)
I agree this minimal change is appropriate as a bug fix.
But I think that it would be good to provide a follow-up
that reworks this code a bit to to use read_poll_timeout().
As per the aim of the cited commit.
This should be targeted at net-next (or iwl-next?).
Once this bug fix propagates to in net-next.
Reviewed-by: Simon Horman <horms@kernel.org>
WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <horms@kernel.org>
To: Ding Hui <dinghui@sangfor.com.cn>
Cc: anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, jacob.e.keller@intel.com,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next] ice: Fix incorrect timeout in ice_release_res()
Date: Fri, 5 Dec 2025 16:16:06 +0000 [thread overview]
Message-ID: <aTMFRkYZGtk3a_EP@horms.kernel.org> (raw)
In-Reply-To: <20251205081609.23091-1-dinghui@sangfor.com.cn>
On Fri, Dec 05, 2025 at 04:16:08PM +0800, Ding Hui wrote:
> 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().
>
> Fixes: 5f6df173f92e ("ice: implement and use rd32_poll_timeout for ice_sq_done timeout")
> Signed-off-by: Ding Hui <dinghui@sangfor.com.cn>
Thanks,
I agree with the analysis above and that the problem was introduced
by the cited commit.
As a fix for code present in net this should probably be targeted
at net (or iwl-net?) rather than net-next. But perhaps there is
no need to repost just to address that.
> ---
> 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)
I agree this minimal change is appropriate as a bug fix.
But I think that it would be good to provide a follow-up
that reworks this code a bit to to use read_poll_timeout().
As per the aim of the cited commit.
This should be targeted at net-next (or iwl-next?).
Once this bug fix propagates to in net-next.
Reviewed-by: Simon Horman <horms@kernel.org>
next prev parent reply other threads:[~2025-12-05 16:16 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-05 8:16 [Intel-wired-lan] [PATCH net-next] ice: Fix incorrect timeout in ice_release_res() Ding Hui
2025-12-05 8:16 ` Ding Hui
2025-12-05 16:16 ` Simon Horman [this message]
2025-12-05 16:16 ` Simon Horman
2025-12-06 1:50 ` [Intel-wired-lan] " Ding Hui
2025-12-06 1:50 ` Ding Hui
2025-12-05 21:09 ` [Intel-wired-lan] " Loktionov, Aleksandr
2025-12-05 21:09 ` Loktionov, Aleksandr
2025-12-06 2:42 ` Ding Hui
2025-12-06 9:46 ` Simon Horman
2025-12-06 13:08 ` Ding Hui
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aTMFRkYZGtk3a_EP@horms.kernel.org \
--to=horms@kernel.org \
--cc=andrew+netdev@lunn.ch \
--cc=anthony.l.nguyen@intel.com \
--cc=davem@davemloft.net \
--cc=dinghui@sangfor.com.cn \
--cc=edumazet@google.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jacob.e.keller@intel.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=przemyslaw.kitszel@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.