lkml.org 
[lkml]   [2019]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled
Date
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

\
 
 \ /
  Last update: 2019-12-16 10:19    [W:0.057 / U:0.712 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site