| Message ID | 20211201014705.6975-3-xiaoliang.yang_1@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | multiple queues support on imx8mp-evk | expand |
On 01.12.2021 02:47, Xiaoliang Yang wrote: > stmmac_tx_timeout() function is called when a queue transmission > timeout. When Strict Priority is used as scheduling algorithms, the > lower priority queue may be blocked by a higher prority queue, which > will lead to tx timeout. We don't want to enable the tx watchdog timeout > in this case. Therefore, this patch make stmmac-tx-timeout configurable. > Your patch just disables the timeout handler, the WARN_ONCE() would still fire. And shouldn't this be a runtime setting rather than a compile-time setting? > This patch set the CONFIG_STMMAC_TX_TIMEOUT by default when STMMAC_ETH > is selected. If anyone want to disable the tx watchdog timeout of > stmmac, he can unset the CONFIG_STMMAC_TX_TIMEOUT in menuconfig. > > Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com> > --- > drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++++++++++++ > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++++ > 2 files changed, 16 insertions(+) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > index 929cfc22cd0c..856c7d056b61 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > @@ -271,4 +271,16 @@ config STMMAC_PCI > If you have a controller with this interface, say Y or M here. > > If unsure, say N. > + > +config STMMAC_TX_TIMEOUT > + bool "STMMAC TX timeout support" > + default STMMAC_ETH > + depends on STMMAC_ETH > + help > + Support for TX timeout enable on stmmac. > + > + This selects the TX watchdog timeout support for stmmac driver. The > + feature is enabled by default when STMMAC_ETH is selected. If you > + want to disable the TX watchdog timeout feature, say N here. > + > endif > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 89a6c35e2546..0a712b5d0715 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -5421,6 +5421,7 @@ static int stmmac_napi_poll_rxtx(struct napi_struct *napi, int budget) > return min(rxtx_done, budget - 1); > } > > +#ifdef CONFIG_STMMAC_TX_TIMEOUT > /** > * stmmac_tx_timeout > * @dev : Pointer to net device structure > @@ -5436,6 +5437,7 @@ static void stmmac_tx_timeout(struct net_device *dev, unsigned int txqueue) > > stmmac_global_err(priv); > } > +#endif > > /** > * stmmac_set_rx_mode - entry point for multicast addressing > @@ -6632,7 +6634,9 @@ static const struct net_device_ops stmmac_netdev_ops = { > .ndo_fix_features = stmmac_fix_features, > .ndo_set_features = stmmac_set_features, > .ndo_set_rx_mode = stmmac_set_rx_mode, > +#ifdef CONFIG_STMMAC_TX_TIMEOUT > .ndo_tx_timeout = stmmac_tx_timeout, > +#endif > .ndo_eth_ioctl = stmmac_ioctl, > .ndo_setup_tc = stmmac_setup_tc, > .ndo_select_queue = stmmac_select_queue, >
Hi Heiner, On Dec 02, 2021 at 16:13:20, Heiner Kallweit wrote: > > stmmac_tx_timeout() function is called when a queue transmission > > timeout. When Strict Priority is used as scheduling algorithms, the > > lower priority queue may be blocked by a higher prority queue, which > > will lead to tx timeout. We don't want to enable the tx watchdog > > timeout in this case. Therefore, this patch make stmmac-tx-timeout > configurable. > > > Your patch just disables the timeout handler, the WARN_ONCE() would still fire. > And shouldn't this be a runtime setting rather than a compile-time setting? > I tried to disable it in stmmac_tx_timeout() at runtime, the WARN_ONCE() will still be called from dev_watchdog() in sch_generic.c. It seems only when the timeout handler is NULL can disable the netdev watchdog up. So I did this in compile-time setting. Regards, Xiaoliang
On 02.12.2021 11:28, Xiaoliang Yang wrote: > Hi Heiner, > > On Dec 02, 2021 at 16:13:20, Heiner Kallweit wrote: >>> stmmac_tx_timeout() function is called when a queue transmission >>> timeout. When Strict Priority is used as scheduling algorithms, the >>> lower priority queue may be blocked by a higher prority queue, which >>> will lead to tx timeout. We don't want to enable the tx watchdog >>> timeout in this case. Therefore, this patch make stmmac-tx-timeout >> configurable. >>> >> Your patch just disables the timeout handler, the WARN_ONCE() would still fire. >> And shouldn't this be a runtime setting rather than a compile-time setting? >> > I tried to disable it in stmmac_tx_timeout() at runtime, the WARN_ONCE() will still be called from dev_watchdog() in sch_generic.c. It seems only when the timeout handler is NULL can disable the netdev watchdog up. So I did this in compile-time setting. > The issue you're trying to fix is not driver-specific. Therefore the solution should be added to net core. > Regards, > Xiaoliang >
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig index 929cfc22cd0c..856c7d056b61 100644 --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig @@ -271,4 +271,16 @@ config STMMAC_PCI If you have a controller with this interface, say Y or M here. If unsure, say N. + +config STMMAC_TX_TIMEOUT + bool "STMMAC TX timeout support" + default STMMAC_ETH + depends on STMMAC_ETH + help + Support for TX timeout enable on stmmac. + + This selects the TX watchdog timeout support for stmmac driver. The + feature is enabled by default when STMMAC_ETH is selected. If you + want to disable the TX watchdog timeout feature, say N here. + endif diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 89a6c35e2546..0a712b5d0715 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -5421,6 +5421,7 @@ static int stmmac_napi_poll_rxtx(struct napi_struct *napi, int budget) return min(rxtx_done, budget - 1); } +#ifdef CONFIG_STMMAC_TX_TIMEOUT /** * stmmac_tx_timeout * @dev : Pointer to net device structure @@ -5436,6 +5437,7 @@ static void stmmac_tx_timeout(struct net_device *dev, unsigned int txqueue) stmmac_global_err(priv); } +#endif /** * stmmac_set_rx_mode - entry point for multicast addressing @@ -6632,7 +6634,9 @@ static const struct net_device_ops stmmac_netdev_ops = { .ndo_fix_features = stmmac_fix_features, .ndo_set_features = stmmac_set_features, .ndo_set_rx_mode = stmmac_set_rx_mode, +#ifdef CONFIG_STMMAC_TX_TIMEOUT .ndo_tx_timeout = stmmac_tx_timeout, +#endif .ndo_eth_ioctl = stmmac_ioctl, .ndo_setup_tc = stmmac_setup_tc, .ndo_select_queue = stmmac_select_queue,
stmmac_tx_timeout() function is called when a queue transmission timeout. When Strict Priority is used as scheduling algorithms, the lower priority queue may be blocked by a higher prority queue, which will lead to tx timeout. We don't want to enable the tx watchdog timeout in this case. Therefore, this patch make stmmac-tx-timeout configurable. This patch set the CONFIG_STMMAC_TX_TIMEOUT by default when STMMAC_ETH is selected. If anyone want to disable the tx watchdog timeout of stmmac, he can unset the CONFIG_STMMAC_TX_TIMEOUT in menuconfig. Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com> --- drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++++++++++++ drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++++ 2 files changed, 16 insertions(+)