RE: [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled

From: Jakub Kicinski <redacted>
Date: Dec/14/2019, 20:36:23 (UTC+00:00)

On Tue, 10 Dec 2019 20:54:43 +0100, Jose Abreu wrote:

quoted

@@ -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!

quoted

+	}
 }
 
 /**

quoted

@@ -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