Messages in this thread | | | Date | Sat, 14 Dec 2019 12:36:23 -0800 | From | Jakub Kicinski <> | Subject | Re: [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled |
| |
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?
> + } > } > > /**
> @@ -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.
> + unsigned long flags; > > - /* Force transmission restart */ > - tx_q = &priv->tx_queue[chan]; > - if (tx_q->cur_tx != tx_q->dirty_tx) { > - stmmac_enable_dma_transmission(priv, priv->ioaddr); > - stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, > - chan); > + spin_lock_irqsave(&ch->lock, flags); > + stmmac_enable_dma_irq(priv, priv->ioaddr, chan, 0, 1); > + spin_unlock_irqrestore(&ch->lock, flags); > } > > return work_done;
| |