Messages in this thread | | | From | Jose Abreu <> | Subject | RE: [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled | Date | Mon, 16 Dec 2019 09:18:50 +0000 |
| |
From: Jakub Kicinski <jakub.kicinski@netronome.com> Date: Dec/14/2019, 20:36:23 (UTC+00:00)
> On Tue, 10 Dec 2019 20:54:43 +0100, Jose Abreu wrote: > > @@ -2278,10 +2286,14 @@ static void stmmac_tx_timer(struct timer_list *t) > > * If NAPI is already running we can miss some events. Let's rearm > > * the timer and try again. > > */ > > - if (likely(napi_schedule_prep(&ch->tx_napi))) > > + if (likely(napi_schedule_prep(&ch->tx_napi))) { > > + unsigned long flags; > > + > > + spin_lock_irqsave(&ch->lock, flags); > > + stmmac_disable_dma_irq(priv, priv->ioaddr, ch->index, 0, 1); > > + spin_unlock_irqrestore(&ch->lock, flags); > > __napi_schedule(&ch->tx_napi); > > - else > > - mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10)); > > You should also remove the comment above the if statement if it's > really okay to no longer re-arm the timer. No?
Yeah, agreed!
> > > + } > > } > > > > /** > > > @@ -3759,24 +3777,18 @@ static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget) > > struct stmmac_channel *ch = > > container_of(napi, struct stmmac_channel, tx_napi); > > struct stmmac_priv *priv = ch->priv_data; > > - struct stmmac_tx_queue *tx_q; > > u32 chan = ch->index; > > int work_done; > > > > priv->xstats.napi_poll++; > > > > - work_done = stmmac_tx_clean(priv, DMA_TX_SIZE, chan); > > - work_done = min(work_done, budget); > > - > > - if (work_done < budget) > > - napi_complete_done(napi, work_done); > > + work_done = stmmac_tx_clean(priv, budget, chan); > > + if (work_done < budget && napi_complete_done(napi, work_done)) { > > Not really related to this patch, but this looks a little suspicious. > I think the TX completions should all be processed regardless of the > budget. The budget is for RX.
Well but this is a TX NAPI ... Shouldn't it be limited to prevent CPU starvation ?
--- Thanks, Jose Miguel Abreu
| |