| Message ID | 20211210193556.1349090-3-yannick.vignon@oss.nxp.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: Improving network scheduling latencies | expand |
Hi Yannick, > -----Original Message----- > From: Yannick Vignon (OSS) <yannick.vignon@oss.nxp.com> > Sent: 2021年12月11日 3:36 > To: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue > <alexandre.torgue@st.com>; netdev@vger.kernel.org; Ong Boon Leong > <boon.leong.ong@intel.com>; David S. Miller <davem@davemloft.net>; > Jakub Kicinski <kuba@kernel.org>; Jose Abreu <joabreu@synopsys.com>; > Eric Dumazet <edumazet@google.com>; Wei Wang <weiwan@google.com>; > Alexander Lobakin <alexandr.lobakin@intel.com>; Vladimir Oltean > <olteanv@gmail.com>; Xiaoliang Yang <xiaoliang.yang_1@nxp.com>; > Mingkai Hu <mingkai.hu@nxp.com>; Joakim Zhang > <qiangqing.zhang@nxp.com>; Sebastien Laveze > <sebastien.laveze@nxp.com> > Cc: Yannick Vignon <yannick.vignon@nxp.com> > Subject: [RFC net-next 2/4] net: stmmac: move to threaded IRQ > > From: Yannick Vignon <yannick.vignon@nxp.com> > > WIP (seems to generate warnings/error on startup) > > When an IRQ is forced threaded, execution of the handler remains protected > by local_bh_disable()/local_bh_enable() calls to keep the semantics of the > IRQ context and avoid deadlocks. However, this also creates a contention > point where a higher prio interrupt handler gets blocked by a lower prio task > already holding the lock. Even though priority inheritance kicks in in such a > case, the lower prio task can still execute for an indefinite time. > > Move the stmmac interrupts to be explicitely threaded, so that high priority > traffic can be processed without delay even if another piece of code was > already running with BH disabled. > > Signed-off-by: Yannick Vignon <yannick.vignon@nxp.com> > --- > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 28 +++++++++---------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 748195697e5a..8bf24902be3c 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -3460,8 +3460,8 @@ static int stmmac_request_irq_multi_msi(struct > net_device *dev) > /* For common interrupt */ > int_name = priv->int_name_mac; > sprintf(int_name, "%s:%s", dev->name, "mac"); > - ret = request_irq(dev->irq, stmmac_mac_interrupt, > - 0, int_name, dev); > + ret = request_threaded_irq(dev->irq, NULL, stmmac_interrupt, > + IRQF_ONESHOT, int_name, dev); Why change from stmmac_mac_interrupt() to stmmac_interrupt()? A copy-paste issue? Best Regards, Joakim Zhang > if (unlikely(ret < 0)) { > netdev_err(priv->dev, > "%s: alloc mac MSI %d (error: %d)\n", @@ -3476,9 +3476,9 > @@ static int stmmac_request_irq_multi_msi(struct net_device *dev) > if (priv->wol_irq > 0 && priv->wol_irq != dev->irq) { > int_name = priv->int_name_wol; > sprintf(int_name, "%s:%s", dev->name, "wol"); > - ret = request_irq(priv->wol_irq, > - stmmac_mac_interrupt, > - 0, int_name, dev); > + ret = request_threaded_irq(priv->wol_irq, > + NULL, stmmac_mac_interrupt, > + IRQF_ONESHOT, int_name, dev); > if (unlikely(ret < 0)) { > netdev_err(priv->dev, > "%s: alloc wol MSI %d (error: %d)\n", @@ -3494,9 > +3494,9 @@ static int stmmac_request_irq_multi_msi(struct net_device > *dev) > if (priv->lpi_irq > 0 && priv->lpi_irq != dev->irq) { > int_name = priv->int_name_lpi; > sprintf(int_name, "%s:%s", dev->name, "lpi"); > - ret = request_irq(priv->lpi_irq, > - stmmac_mac_interrupt, > - 0, int_name, dev); > + ret = request_threaded_irq(priv->lpi_irq, > + NULL, stmmac_mac_interrupt, > + IRQF_ONESHOT, int_name, dev); > if (unlikely(ret < 0)) { > netdev_err(priv->dev, > "%s: alloc lpi MSI %d (error: %d)\n", @@ -3605,8 > +3605,8 @@ static int stmmac_request_irq_single(struct net_device *dev) > enum request_irq_err irq_err; > int ret; > > - ret = request_irq(dev->irq, stmmac_interrupt, > - IRQF_SHARED, dev->name, dev); > + ret = request_threaded_irq(dev->irq, NULL, stmmac_interrupt, > + IRQF_SHARED | IRQF_ONESHOT, dev->name, dev); > if (unlikely(ret < 0)) { > netdev_err(priv->dev, > "%s: ERROR: allocating the IRQ %d (error: %d)\n", @@ > -3619,8 +3619,8 @@ static int stmmac_request_irq_single(struct net_device > *dev) > * is used for WoL > */ > if (priv->wol_irq > 0 && priv->wol_irq != dev->irq) { > - ret = request_irq(priv->wol_irq, stmmac_interrupt, > - IRQF_SHARED, dev->name, dev); > + ret = request_threaded_irq(priv->wol_irq, NULL, > stmmac_interrupt, > + IRQF_SHARED | IRQF_ONESHOT, dev->name, dev); > if (unlikely(ret < 0)) { > netdev_err(priv->dev, > "%s: ERROR: allocating the WoL IRQ %d (%d)\n", @@ > -3632,8 +3632,8 @@ static int stmmac_request_irq_single(struct net_device > *dev) > > /* Request the IRQ lines */ > if (priv->lpi_irq > 0 && priv->lpi_irq != dev->irq) { > - ret = request_irq(priv->lpi_irq, stmmac_interrupt, > - IRQF_SHARED, dev->name, dev); > + ret = request_threaded_irq(priv->lpi_irq, NULL, stmmac_interrupt, > + IRQF_SHARED | IRQF_ONESHOT, dev->name, dev); > if (unlikely(ret < 0)) { > netdev_err(priv->dev, > "%s: ERROR: allocating the LPI IRQ %d (%d)\n", > -- > 2.25.1
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 748195697e5a..8bf24902be3c 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3460,8 +3460,8 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev) /* For common interrupt */ int_name = priv->int_name_mac; sprintf(int_name, "%s:%s", dev->name, "mac"); - ret = request_irq(dev->irq, stmmac_mac_interrupt, - 0, int_name, dev); + ret = request_threaded_irq(dev->irq, NULL, stmmac_interrupt, + IRQF_ONESHOT, int_name, dev); if (unlikely(ret < 0)) { netdev_err(priv->dev, "%s: alloc mac MSI %d (error: %d)\n", @@ -3476,9 +3476,9 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev) if (priv->wol_irq > 0 && priv->wol_irq != dev->irq) { int_name = priv->int_name_wol; sprintf(int_name, "%s:%s", dev->name, "wol"); - ret = request_irq(priv->wol_irq, - stmmac_mac_interrupt, - 0, int_name, dev); + ret = request_threaded_irq(priv->wol_irq, + NULL, stmmac_mac_interrupt, + IRQF_ONESHOT, int_name, dev); if (unlikely(ret < 0)) { netdev_err(priv->dev, "%s: alloc wol MSI %d (error: %d)\n", @@ -3494,9 +3494,9 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev) if (priv->lpi_irq > 0 && priv->lpi_irq != dev->irq) { int_name = priv->int_name_lpi; sprintf(int_name, "%s:%s", dev->name, "lpi"); - ret = request_irq(priv->lpi_irq, - stmmac_mac_interrupt, - 0, int_name, dev); + ret = request_threaded_irq(priv->lpi_irq, + NULL, stmmac_mac_interrupt, + IRQF_ONESHOT, int_name, dev); if (unlikely(ret < 0)) { netdev_err(priv->dev, "%s: alloc lpi MSI %d (error: %d)\n", @@ -3605,8 +3605,8 @@ static int stmmac_request_irq_single(struct net_device *dev) enum request_irq_err irq_err; int ret; - ret = request_irq(dev->irq, stmmac_interrupt, - IRQF_SHARED, dev->name, dev); + ret = request_threaded_irq(dev->irq, NULL, stmmac_interrupt, + IRQF_SHARED | IRQF_ONESHOT, dev->name, dev); if (unlikely(ret < 0)) { netdev_err(priv->dev, "%s: ERROR: allocating the IRQ %d (error: %d)\n", @@ -3619,8 +3619,8 @@ static int stmmac_request_irq_single(struct net_device *dev) * is used for WoL */ if (priv->wol_irq > 0 && priv->wol_irq != dev->irq) { - ret = request_irq(priv->wol_irq, stmmac_interrupt, - IRQF_SHARED, dev->name, dev); + ret = request_threaded_irq(priv->wol_irq, NULL, stmmac_interrupt, + IRQF_SHARED | IRQF_ONESHOT, dev->name, dev); if (unlikely(ret < 0)) { netdev_err(priv->dev, "%s: ERROR: allocating the WoL IRQ %d (%d)\n", @@ -3632,8 +3632,8 @@ static int stmmac_request_irq_single(struct net_device *dev) /* Request the IRQ lines */ if (priv->lpi_irq > 0 && priv->lpi_irq != dev->irq) { - ret = request_irq(priv->lpi_irq, stmmac_interrupt, - IRQF_SHARED, dev->name, dev); + ret = request_threaded_irq(priv->lpi_irq, NULL, stmmac_interrupt, + IRQF_SHARED | IRQF_ONESHOT, dev->name, dev); if (unlikely(ret < 0)) { netdev_err(priv->dev, "%s: ERROR: allocating the LPI IRQ %d (%d)\n",