| Message ID | 20240528150339.6791-3-paul.barker.ct@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Improve GbEth performance on Renesas RZ/G2L and related SoCs | expand |
On 5/28/24 6:03 PM, Paul Barker wrote: > Make use of the busypolling status returned from NAPI complete to decide My spellchecker/translator trip over "busypolling" -- consider using "busy-polling"? And did you actually mean napi_complete_done()? > if interrupts shall be re-enabled or not. This is useful to reduce the > interrupt overhead. > > While at it switch to using napi_complete_done() as it take into account Takes. > the work done when providing the busypolling status. Again, "busy-polling"? > Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com> [...] MBR, Sergey
On 5/28/24 7:44 PM, Sergey Shtylyov wrote: >> Make use of the busypolling status returned from NAPI complete to decide > > My spellchecker/translator trip over "busypolling" -- consider using > "busy-polling"? > And did you actually mean napi_complete_done()? Ah, napi_complete() also returns a result... maybe this should be reworded as "NAPI completion"? >> if interrupts shall be re-enabled or not. This is useful to reduce the >> interrupt overhead. >> >> While at it switch to using napi_complete_done() as it take into account > > Takes. > >> the work done when providing the busypolling status. > > Again, "busy-polling"? > >> Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com> [...] MBR, Sergey
On 28/05/2024 17:47, Sergey Shtylyov wrote: > On 5/28/24 7:44 PM, Sergey Shtylyov wrote: > >>> Make use of the busypolling status returned from NAPI complete to decide >> >> My spellchecker/translator trip over "busypolling" -- consider using >> "busy-polling"? >> And did you actually mean napi_complete_done()? > > Ah, napi_complete() also returns a result... maybe this should be reworded > as "NAPI completion"? > >>> if interrupts shall be re-enabled or not. This is useful to reduce the >>> interrupt overhead. >>> >>> While at it switch to using napi_complete_done() as it take into account >> >> Takes. >> >>> the work done when providing the busypolling status. >> >> Again, "busy-polling"? Ack to all of the above. I used the commit message suggested by Niklas here. I'll revise it a little for v5... Thanks,
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index 193ad05383a8..472aa80002be 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -1341,23 +1341,19 @@ static int ravb_poll(struct napi_struct *napi, int budget) if (priv->rx_fifo_errors != ndev->stats.rx_fifo_errors) ndev->stats.rx_fifo_errors = priv->rx_fifo_errors; - if (work_done == budget) - goto out; - - napi_complete(napi); - - /* Re-enable RX/TX interrupts */ - spin_lock_irqsave(&priv->lock, flags); - if (!info->irq_en_dis) { - ravb_modify(ndev, RIC0, mask, mask); - ravb_modify(ndev, TIC, mask, mask); - } else { - ravb_write(ndev, mask, RIE0); - ravb_write(ndev, mask, TIE); + if (work_done < budget && napi_complete_done(napi, work_done)) { + /* Re-enable RX/TX interrupts */ + spin_lock_irqsave(&priv->lock, flags); + if (!info->irq_en_dis) { + ravb_modify(ndev, RIC0, mask, mask); + ravb_modify(ndev, TIC, mask, mask); + } else { + ravb_write(ndev, mask, RIE0); + ravb_write(ndev, mask, TIE); + } + spin_unlock_irqrestore(&priv->lock, flags); } - spin_unlock_irqrestore(&priv->lock, flags); -out: return work_done; }
Make use of the busypolling status returned from NAPI complete to decide if interrupts shall be re-enabled or not. This is useful to reduce the interrupt overhead. While at it switch to using napi_complete_done() as it take into account the work done when providing the busypolling status. Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com> --- Changes v3->v4: * Used Niklas' suggested commit message. drivers/net/ethernet/renesas/ravb_main.c | 26 ++++++++++-------------- 1 file changed, 11 insertions(+), 15 deletions(-)