| Message ID | 20201027225454.3492351-15-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | in_interrupt() cleanup, part 2 | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (8cb17737940b156329cb5210669b9c9b23f4dd56) |
| snowpatch_ozlabs/checkpatch | success | |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On Tue, 27 Oct 2020 23:54:53 +0100 Sebastian Andrzej Siewior wrote: > The driver uses in_irq() + in_serving_softirq() magic to decide if NAPI > scheduling is required or packet processing. > > The usage of in_*() in drivers is phased out and Linus clearly requested > that code which changes behaviour depending on context should either be > seperated or the context be conveyed in an argument passed by the caller, > which usually knows the context. > > Use the `napi' argument passed by the callback. It is set true if > called from the interrupt handler and NAPI should be scheduled. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Cc: "Horia Geantă" <horia.geanta@nxp.com> > Cc: Aymen Sghaier <aymen.sghaier@nxp.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Madalin Bucur <madalin.bucur@nxp.com> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: Li Yang <leoyang.li@nxp.com> > Cc: linux-crypto@vger.kernel.org > Cc: netdev@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Cc: linux-arm-kernel@lists.infradead.org > --- > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > index 27835310b718e..2c949acd74c67 100644 > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > @@ -2300,9 +2300,9 @@ static void dpaa_tx_conf(struct net_device *net_dev, > } > > static inline int dpaa_eth_napi_schedule(struct dpaa_percpu_priv *percpu_priv, > - struct qman_portal *portal) > + struct qman_portal *portal, bool napi) > { > - if (unlikely(in_irq() || !in_serving_softirq())) { > + if (napi) { > /* Disable QMan IRQ and invoke NAPI */ > qman_p_irqsource_remove(portal, QM_PIRQ_DQRI); > Nit: some networking drivers have a bool napi which means "are we running in napi context", the semantics here feel a little backwards, at least to me. But if I'm the only one thinking this, so be it.
On 2020-10-31 10:12:15 [-0700], Jakub Kicinski wrote: > Nit: some networking drivers have a bool napi which means "are we > running in napi context", the semantics here feel a little backwards, > at least to me. But if I'm the only one thinking this, so be it. I renamed it to `sched_napi'. Sebastian
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c index 27835310b718e..2c949acd74c67 100644 --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c @@ -2300,9 +2300,9 @@ static void dpaa_tx_conf(struct net_device *net_dev, } static inline int dpaa_eth_napi_schedule(struct dpaa_percpu_priv *percpu_priv, - struct qman_portal *portal) + struct qman_portal *portal, bool napi) { - if (unlikely(in_irq() || !in_serving_softirq())) { + if (napi) { /* Disable QMan IRQ and invoke NAPI */ qman_p_irqsource_remove(portal, QM_PIRQ_DQRI); @@ -2333,7 +2333,7 @@ static enum qman_cb_dqrr_result rx_error_dqrr(struct qman_portal *portal, percpu_priv = this_cpu_ptr(priv->percpu_priv); - if (dpaa_eth_napi_schedule(percpu_priv, portal)) + if (dpaa_eth_napi_schedule(percpu_priv, portal, napi)) return qman_cb_dqrr_stop; dpaa_eth_refill_bpools(priv); @@ -2377,7 +2377,7 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal, percpu_priv = this_cpu_ptr(priv->percpu_priv); percpu_stats = &percpu_priv->stats; - if (unlikely(dpaa_eth_napi_schedule(percpu_priv, portal))) + if (unlikely(dpaa_eth_napi_schedule(percpu_priv, portal, napi))) return qman_cb_dqrr_stop; /* Make sure we didn't run out of buffers */ @@ -2474,7 +2474,7 @@ static enum qman_cb_dqrr_result conf_error_dqrr(struct qman_portal *portal, percpu_priv = this_cpu_ptr(priv->percpu_priv); - if (dpaa_eth_napi_schedule(percpu_priv, portal)) + if (dpaa_eth_napi_schedule(percpu_priv, portal, napi)) return qman_cb_dqrr_stop; dpaa_tx_error(net_dev, priv, percpu_priv, &dq->fd, fq->fqid); @@ -2499,7 +2499,7 @@ static enum qman_cb_dqrr_result conf_dflt_dqrr(struct qman_portal *portal, percpu_priv = this_cpu_ptr(priv->percpu_priv); - if (dpaa_eth_napi_schedule(percpu_priv, portal)) + if (dpaa_eth_napi_schedule(percpu_priv, portal, napi)) return qman_cb_dqrr_stop; dpaa_tx_conf(net_dev, priv, percpu_priv, &dq->fd, fq->fqid);
The driver uses in_irq() + in_serving_softirq() magic to decide if NAPI scheduling is required or packet processing. The usage of in_*() in drivers is phased out and Linus clearly requested that code which changes behaviour depending on context should either be seperated or the context be conveyed in an argument passed by the caller, which usually knows the context. Use the `napi' argument passed by the callback. It is set true if called from the interrupt handler and NAPI should be scheduled. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Cc: "Horia Geantă" <horia.geanta@nxp.com> Cc: Aymen Sghaier <aymen.sghaier@nxp.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Cc: Madalin Bucur <madalin.bucur@nxp.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Li Yang <leoyang.li@nxp.com> Cc: linux-crypto@vger.kernel.org Cc: netdev@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-arm-kernel@lists.infradead.org --- drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)