net: netdev_queue: netdev_txq_completed_mb(): fix wake condition

Message ID 20240112-netdev_queue-v1-1-102c2d57e20a@pengutronix.de
State New
Headers
Series net: netdev_queue: netdev_txq_completed_mb(): fix wake condition |

Commit Message

Marc Kleine-Budde Jan. 12, 2024, 4:13 p.m. UTC
  netif_txq_try_stop() uses "get_desc >= start_thrs" as the check for
the call to netif_tx_start_queue().

Use ">=" i netdev_txq_completed_mb(), too.

Fixes: c91c46de6bbc ("net: provide macros for commonly copied lockless queue stop/wake code")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Hello,

I'm currently converting a networking driver with a TX-FIFO depth of
1 (CAN device with lots of errata :/) to the netdev_queue.h helpers
and stumbled over an off-by-one error on __netif_txq_completed_wake().

regards,
Marc
---
 include/net/netdev_queues.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: 907ee6681788556b9ade3ad0a1f6f4aea192399c
change-id: 20240112-netdev_queue-e91686f5fece

Best regards,
  

Comments

Jakub Kicinski Jan. 13, 2024, 2:44 a.m. UTC | #1
On Fri, 12 Jan 2024 17:13:14 +0100 Marc Kleine-Budde wrote:
> netif_txq_try_stop() uses "get_desc >= start_thrs" as the check for
> the call to netif_tx_start_queue().
> 
> Use ">=" i netdev_txq_completed_mb(), too.
> 
> Fixes: c91c46de6bbc ("net: provide macros for commonly copied lockless queue stop/wake code")
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> I'm currently converting a networking driver with a TX-FIFO depth of
> 1 (CAN device with lots of errata :/) to the netdev_queue.h helpers
> and stumbled over an off-by-one error on __netif_txq_completed_wake().

Makes sense, could be copy'n'paste from one of the drivers this is
based on. A bit unsure if it deserves the Fixes tag and net as we don't
know of any current user that would be suffering. start_thrs == ring size
is a bit of an extreme use case indeed :) Either way:

Acked-by: Jakub Kicinski <kuba@kernel.org>
  
patchwork-bot+netdevbpf@kernel.org Jan. 13, 2024, 6:30 p.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 12 Jan 2024 17:13:14 +0100 you wrote:
> netif_txq_try_stop() uses "get_desc >= start_thrs" as the check for
> the call to netif_tx_start_queue().
> 
> Use ">=" i netdev_txq_completed_mb(), too.
> 
> Fixes: c91c46de6bbc ("net: provide macros for commonly copied lockless queue stop/wake code")
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> [...]

Here is the summary with links:
  - net: netdev_queue: netdev_txq_completed_mb(): fix wake condition
    https://git.kernel.org/netdev/net/c/894d7508316e

You are awesome, thank you!
  

Patch

diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h
index d68b0a483431..8b8ed4e13d74 100644
--- a/include/net/netdev_queues.h
+++ b/include/net/netdev_queues.h
@@ -128,7 +128,7 @@  netdev_txq_completed_mb(struct netdev_queue *dev_queue,
 		netdev_txq_completed_mb(txq, pkts, bytes);		\
 									\
 		_res = -1;						\
-		if (pkts && likely(get_desc > start_thrs)) {		\
+		if (pkts && likely(get_desc >= start_thrs)) {		\
 			_res = 1;					\
 			if (unlikely(netif_tx_queue_stopped(txq)) &&	\
 			    !(down_cond)) {				\