From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A7198C3DA6E for ; Fri, 5 Jan 2024 09:46:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=7pdVu8jHcDRSz67J4dhEb7d+Dgx/ZgLGqkaOus5Fc1w=; b=ptLMBW7TYDEuoK I8Q8FFYYeLtSY63F86XCU847OEFBzWJNoctHjGvP0i2hsqvtcGQJKdMpqVwYpUTzbZK12bQIJVvLV 5GSJEOVIYaf0UYyCALQk8m6NewR2kNTM4yjsdW4CkGXNV9sIfegXQIP/TunP6WPdaai1h/sVbnGaM EwrXS0fkhkeIwdgDTE61MvbZXQ2LcVNR6qkte4p7JL/79G6+z3FZIJ1b4WL2SqGNDiIwRUWeevpqj m4tcaGE6EYhUqYrJJIOmRSsjLLfuc78w/2MqV02lWACnAEVijFg2S5IBlfpuFj9TUl9DLwWhYJA+Z AzpPzU6Vh3TCHArS6Gaw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rLgmD-00GQun-11; Fri, 05 Jan 2024 09:46:05 +0000 Received: from mail-wr1-x434.google.com ([2a00:1450:4864:20::434]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rLgm8-00GQsf-2H for linux-arm-kernel@lists.infradead.org; Fri, 05 Jan 2024 09:46:03 +0000 Received: by mail-wr1-x434.google.com with SMTP id ffacd0b85a97d-336755f1688so1177980f8f.0 for ; Fri, 05 Jan 2024 01:45:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20230601.gappssmtp.com; s=20230601; t=1704447957; x=1705052757; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=6dQ2Zcf3LUGtuQQ4BG4OyEdzBvj9Rr7K+Ifz60LIvyE=; b=L2Yil1wFkTwFXpTgkeLfgKItOz9ZKmzyMA3RzjUCf8+kMalbcl7gd0h+3k45Qz5BVe e+7RRsFlXEbUmd7qNqEIgHeugdkSAsX1v5mBXNaIREolG8YcFXv6FV0WZpi71woAk9p8 H6LN+q4pha47w5GsjHwklEzejz9KRT/gKNENxeEcnJSJTycik6oW/5NK7GMH57rQSAO5 vz5TauTeIylESpvqkQWE1vomV7koJ3YA4oSNt+5PVw16+kiuDcsFPSdNwDBMAwxIoxOl PWTsuifJcQ5vfSwug0s1Z6lWnDeqwIZv3esi2yka1UWfKf+8E3+e/k3BQvLgKVZLzqdG 9o2Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704447957; x=1705052757; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=6dQ2Zcf3LUGtuQQ4BG4OyEdzBvj9Rr7K+Ifz60LIvyE=; b=Zf98Ha9JAPSf2fFXb4aJGRBUsWwGIkRVvfuepXcAmjuR/xaW+LZ2YRAyYlb3pUg+6g 8nLFj0lXGTNiCo7u/2F5kGlaJOqH+saiQiUeHNlpoi7VZ7tbS5snHmwpEffrbZXEW6jz gLp9wmu3KgVViE/kqFozc0vedQIBWfF3XOOybaEykl2Xr0kdMUELRjbgWxLCuIperqNv kZVDtxwKk9FE+jrb5GusyFv9gXogkaEocRqr6GfhLUVKRz6p3zHaM2vAq+9aKfEykZ39 zWmage+Hbq73A+pQddsXAodDhb+usZXQ1cBavIdtSI4zPRICpL6c4dw6pLSl7/fsORGn BuIQ== X-Gm-Message-State: AOJu0Ywg1YnGenNkq58UL4Alq67rzgGc/Vh9lV8z8klwmPC166X/t5F0 XhU2pXEpfVCKZGiBKNjYvCY8dUYm1nqofQ== X-Google-Smtp-Source: AGHT+IE7xcKFhc+fZICfgIHf6ynVm6myXKHOu6CaW+r5Tx/ljZlkvJMf+DQE7zJ/0LUT9GlPaykdwg== X-Received: by 2002:a5d:4f83:0:b0:337:5a0e:1195 with SMTP id d3-20020a5d4f83000000b003375a0e1195mr292860wru.127.1704447957443; Fri, 05 Jan 2024 01:45:57 -0800 (PST) Received: from localhost (host-213-179-129-39.customer.m-online.net. [213.179.129.39]) by smtp.gmail.com with ESMTPSA id t1-20020adfd001000000b0033672971fabsm1049226wrh.115.2024.01.05.01.45.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Jan 2024 01:45:56 -0800 (PST) Date: Fri, 5 Jan 2024 10:45:55 +0100 From: Jiri Pirko To: Petr Tesarik Cc: Alexandre Torgue , Jose Abreu , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , "open list:STMMAC ETHERNET DRIVER" , "moderated list:ARM/STM32 ARCHITECTURE" , "moderated list:ARM/STM32 ARCHITECTURE" , open list , "open list:ARM/Allwinner sunXi SoC support" Subject: Re: [PATCH] net: stmmac: protect statistics updates with a spinlock Message-ID: References: <20240105091556.15516-1-petr@tesarici.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240105091556.15516-1-petr@tesarici.cz> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240105_014600_747771_33EB2E1B X-CRM114-Status: GOOD ( 17.59 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Fri, Jan 05, 2024 at 10:15:56AM CET, petr@tesarici.cz wrote: >Add a spinlock to fix race conditions while updating Tx/Rx statistics. > >As explained by a comment in , write side of struct >u64_stats_sync must ensure mutual exclusion, or one seqcount update could >be lost on 32-bit platforms, thus blocking readers forever. > >Such lockups have been actually observed on 32-bit Arm after stmmac_xmit() >on one core raced with stmmac_napi_poll_tx() on another core. > >Signed-off-by: Petr Tesarik >--- > drivers/net/ethernet/stmicro/stmmac/common.h | 2 + > .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 4 + > .../net/ethernet/stmicro/stmmac/dwmac4_lib.c | 4 + > .../net/ethernet/stmicro/stmmac/dwmac_lib.c | 4 + > .../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 4 + > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 80 +++++++++++++------ > 6 files changed, 72 insertions(+), 26 deletions(-) > >diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h >index e3f650e88f82..9a17dfc1055d 100644 >--- a/drivers/net/ethernet/stmicro/stmmac/common.h >+++ b/drivers/net/ethernet/stmicro/stmmac/common.h >@@ -70,6 +70,7 @@ struct stmmac_txq_stats { > u64 tx_tso_frames; > u64 tx_tso_nfrags; > struct u64_stats_sync syncp; >+ spinlock_t lock; /* mutual writer exclusion */ > } ____cacheline_aligned_in_smp; > > struct stmmac_rxq_stats { >@@ -79,6 +80,7 @@ struct stmmac_rxq_stats { > u64 rx_normal_irq_n; > u64 napi_poll; > struct u64_stats_sync syncp; >+ spinlock_t lock; /* mutual writer exclusion */ > } ____cacheline_aligned_in_smp; > > /* Extra statistic and debug information exposed by ethtool */ >diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >index 137741b94122..9c568996321d 100644 >--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >@@ -455,9 +455,11 @@ static int sun8i_dwmac_dma_interrupt(struct stmmac_priv *priv, > > if (v & EMAC_TX_INT) { > ret |= handle_tx; >+ spin_lock(&txq_stats->lock); > u64_stats_update_begin(&txq_stats->syncp); > txq_stats->tx_normal_irq_n++; > u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock(&txq_stats->lock); > } > > if (v & EMAC_TX_DMA_STOP_INT) >@@ -479,9 +481,11 @@ static int sun8i_dwmac_dma_interrupt(struct stmmac_priv *priv, > > if (v & EMAC_RX_INT) { > ret |= handle_rx; >+ spin_lock(&rxq_stats->lock); > u64_stats_update_begin(&rxq_stats->syncp); > rxq_stats->rx_normal_irq_n++; > u64_stats_update_end(&rxq_stats->syncp); >+ spin_unlock(&rxq_stats->lock); > } > > if (v & EMAC_RX_BUF_UA_INT) >diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c >index 9470d3fd2ded..e50e8b07724b 100644 >--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c >+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c >@@ -201,15 +201,19 @@ int dwmac4_dma_interrupt(struct stmmac_priv *priv, void __iomem *ioaddr, > } > /* TX/RX NORMAL interrupts */ > if (likely(intr_status & DMA_CHAN_STATUS_RI)) { >+ spin_lock(&rxq_stats->lock); > u64_stats_update_begin(&rxq_stats->syncp); > rxq_stats->rx_normal_irq_n++; > u64_stats_update_end(&rxq_stats->syncp); >+ spin_unlock(&rxq_stats->lock); > ret |= handle_rx; > } > if (likely(intr_status & DMA_CHAN_STATUS_TI)) { >+ spin_lock(&txq_stats->lock); > u64_stats_update_begin(&txq_stats->syncp); > txq_stats->tx_normal_irq_n++; > u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock(&txq_stats->lock); > ret |= handle_tx; > } > >diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c >index 7907d62d3437..a43396a7f852 100644 >--- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c >+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c >@@ -215,16 +215,20 @@ int dwmac_dma_interrupt(struct stmmac_priv *priv, void __iomem *ioaddr, > u32 value = readl(ioaddr + DMA_INTR_ENA); > /* to schedule NAPI on real RIE event. */ > if (likely(value & DMA_INTR_ENA_RIE)) { >+ spin_lock(&rxq_stats->lock); > u64_stats_update_begin(&rxq_stats->syncp); > rxq_stats->rx_normal_irq_n++; > u64_stats_update_end(&rxq_stats->syncp); >+ spin_unlock(&rxq_stats->lock); > ret |= handle_rx; > } > } > if (likely(intr_status & DMA_STATUS_TI)) { >+ spin_lock(&txq_stats->lock); > u64_stats_update_begin(&txq_stats->syncp); > txq_stats->tx_normal_irq_n++; > u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock(&txq_stats->lock); > ret |= handle_tx; > } > if (unlikely(intr_status & DMA_STATUS_ERI)) >diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c >index 3cde695fec91..f4e01436d4cc 100644 >--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c >+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c >@@ -367,15 +367,19 @@ static int dwxgmac2_dma_interrupt(struct stmmac_priv *priv, > /* TX/RX NORMAL interrupts */ > if (likely(intr_status & XGMAC_NIS)) { > if (likely(intr_status & XGMAC_RI)) { >+ spin_lock(&rxq_stats->lock); > u64_stats_update_begin(&rxq_stats->syncp); > rxq_stats->rx_normal_irq_n++; > u64_stats_update_end(&rxq_stats->syncp); >+ spin_unlock(&rxq_stats->lock); > ret |= handle_rx; > } > if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) { >+ spin_lock(&txq_stats->lock); > u64_stats_update_begin(&txq_stats->syncp); > txq_stats->tx_normal_irq_n++; > u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock(&txq_stats->lock); > ret |= handle_tx; > } > } >diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >index 37e64283f910..82d8db04d0d1 100644 >--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >@@ -2515,9 +2515,11 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget) > tx_q->cur_tx = STMMAC_GET_ENTRY(tx_q->cur_tx, priv->dma_conf.dma_tx_size); > entry = tx_q->cur_tx; > } >- flags = u64_stats_update_begin_irqsave(&txq_stats->syncp); >+ spin_lock_irqsave(&txq_stats->lock, flags); >+ u64_stats_update_begin(&txq_stats->syncp); > txq_stats->tx_set_ic_bit += tx_set_ic_bit; >- u64_stats_update_end_irqrestore(&txq_stats->syncp, flags); >+ u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock_irqrestore(&txq_stats->lock, flags); > > if (tx_desc) { > stmmac_flush_tx_descriptors(priv, queue); >@@ -2721,11 +2723,13 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue, > if (tx_q->dirty_tx != tx_q->cur_tx) > *pending_packets = true; > >- flags = u64_stats_update_begin_irqsave(&txq_stats->syncp); >+ spin_lock_irqsave(&txq_stats->lock, flags); >+ u64_stats_update_begin(&txq_stats->syncp); > txq_stats->tx_packets += tx_packets; > txq_stats->tx_pkt_n += tx_packets; > txq_stats->tx_clean++; >- u64_stats_update_end_irqrestore(&txq_stats->syncp, flags); >+ u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock_irqrestore(&txq_stats->lock, flags); > > priv->xstats.tx_errors += tx_errors; > >@@ -4311,13 +4315,15 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev) > netif_tx_stop_queue(netdev_get_tx_queue(priv->dev, queue)); > } > >- flags = u64_stats_update_begin_irqsave(&txq_stats->syncp); >+ spin_lock_irqsave(&txq_stats->lock, flags); >+ u64_stats_update_begin(&txq_stats->syncp); > txq_stats->tx_bytes += skb->len; > txq_stats->tx_tso_frames++; > txq_stats->tx_tso_nfrags += nfrags; > if (set_ic) > txq_stats->tx_set_ic_bit++; >- u64_stats_update_end_irqrestore(&txq_stats->syncp, flags); >+ u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock_irqrestore(&txq_stats->lock, flags); > > if (priv->sarc_type) > stmmac_set_desc_sarc(priv, first, priv->sarc_type); >@@ -4560,11 +4566,13 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) > netif_tx_stop_queue(netdev_get_tx_queue(priv->dev, queue)); > } > >- flags = u64_stats_update_begin_irqsave(&txq_stats->syncp); >+ spin_lock_irqsave(&txq_stats->lock, flags); >+ u64_stats_update_begin(&txq_stats->syncp); > txq_stats->tx_bytes += skb->len; > if (set_ic) > txq_stats->tx_set_ic_bit++; >- u64_stats_update_end_irqrestore(&txq_stats->syncp, flags); >+ u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock_irqrestore(&txq_stats->lock, flags); > > if (priv->sarc_type) > stmmac_set_desc_sarc(priv, first, priv->sarc_type); >@@ -4831,9 +4839,11 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue, > unsigned long flags; > tx_q->tx_count_frames = 0; > stmmac_set_tx_ic(priv, tx_desc); >- flags = u64_stats_update_begin_irqsave(&txq_stats->syncp); >+ spin_lock_irqsave(&txq_stats->lock, flags); >+ u64_stats_update_begin(&txq_stats->syncp); > txq_stats->tx_set_ic_bit++; >- u64_stats_update_end_irqrestore(&txq_stats->syncp, flags); >+ u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock_irqrestore(&txq_stats->lock, flags); > } > > stmmac_enable_dma_transmission(priv, priv->ioaddr); >@@ -5008,10 +5018,12 @@ static void stmmac_dispatch_skb_zc(struct stmmac_priv *priv, u32 queue, > skb_record_rx_queue(skb, queue); > napi_gro_receive(&ch->rxtx_napi, skb); > >- flags = u64_stats_update_begin_irqsave(&rxq_stats->syncp); >+ spin_lock_irqsave(&rxq_stats->lock, flags); >+ u64_stats_update_begin(&rxq_stats->syncp); > rxq_stats->rx_pkt_n++; > rxq_stats->rx_bytes += len; >- u64_stats_update_end_irqrestore(&rxq_stats->syncp, flags); >+ u64_stats_update_end(&rxq_stats->syncp); >+ spin_unlock_irqrestore(&rxq_stats->lock, flags); > } > > static bool stmmac_rx_refill_zc(struct stmmac_priv *priv, u32 queue, u32 budget) >@@ -5248,9 +5260,11 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue) > > stmmac_finalize_xdp_rx(priv, xdp_status); > >- flags = u64_stats_update_begin_irqsave(&rxq_stats->syncp); >+ spin_lock_irqsave(&rxq_stats->lock, flags); >+ u64_stats_update_begin(&rxq_stats->syncp); > rxq_stats->rx_pkt_n += count; >- u64_stats_update_end_irqrestore(&rxq_stats->syncp, flags); >+ u64_stats_update_end(&rxq_stats->syncp); >+ spin_unlock_irqrestore(&rxq_stats->lock, flags); > > priv->xstats.rx_dropped += rx_dropped; > priv->xstats.rx_errors += rx_errors; >@@ -5541,11 +5555,13 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) > > stmmac_rx_refill(priv, queue); > >- flags = u64_stats_update_begin_irqsave(&rxq_stats->syncp); >+ spin_lock_irqsave(&rxq_stats->lock, flags); >+ u64_stats_update_begin(&rxq_stats->syncp); > rxq_stats->rx_packets += rx_packets; > rxq_stats->rx_bytes += rx_bytes; > rxq_stats->rx_pkt_n += count; >- u64_stats_update_end_irqrestore(&rxq_stats->syncp, flags); >+ u64_stats_update_end(&rxq_stats->syncp); >+ spin_unlock_irqrestore(&rxq_stats->lock, flags); > > priv->xstats.rx_dropped += rx_dropped; > priv->xstats.rx_errors += rx_errors; >@@ -5564,9 +5580,11 @@ static int stmmac_napi_poll_rx(struct napi_struct *napi, int budget) > int work_done; > > rxq_stats = &priv->xstats.rxq_stats[chan]; >- flags = u64_stats_update_begin_irqsave(&rxq_stats->syncp); >+ spin_lock_irqsave(&rxq_stats->lock, flags); >+ u64_stats_update_begin(&rxq_stats->syncp); > rxq_stats->napi_poll++; >- u64_stats_update_end_irqrestore(&rxq_stats->syncp, flags); >+ u64_stats_update_end(&rxq_stats->syncp); >+ spin_unlock_irqrestore(&rxq_stats->lock, flags); > > work_done = stmmac_rx(priv, budget, chan); > if (work_done < budget && napi_complete_done(napi, work_done)) { >@@ -5592,9 +5610,11 @@ static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget) > int work_done; > > txq_stats = &priv->xstats.txq_stats[chan]; >- flags = u64_stats_update_begin_irqsave(&txq_stats->syncp); >+ spin_lock_irqsave(&txq_stats->lock, flags); >+ u64_stats_update_begin(&txq_stats->syncp); > txq_stats->napi_poll++; >- u64_stats_update_end_irqrestore(&txq_stats->syncp, flags); >+ u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock_irqrestore(&txq_stats->lock, flags); > > work_done = stmmac_tx_clean(priv, budget, chan, &pending_packets); > work_done = min(work_done, budget); >@@ -5627,14 +5647,18 @@ static int stmmac_napi_poll_rxtx(struct napi_struct *napi, int budget) > unsigned long flags; > > rxq_stats = &priv->xstats.rxq_stats[chan]; >- flags = u64_stats_update_begin_irqsave(&rxq_stats->syncp); >+ spin_lock_irqsave(&rxq_stats->lock, flags); >+ u64_stats_update_begin(&rxq_stats->syncp); > rxq_stats->napi_poll++; >- u64_stats_update_end_irqrestore(&rxq_stats->syncp, flags); >+ u64_stats_update_end(&rxq_stats->syncp); >+ spin_unlock(&rxq_stats->lock); Nitpick: I know that the original code does that, but any idea why u64_stats_update_end_irqrestore() is called here when u64_stats_update_begin_irqsave() is called 2 lines below? IIUC, this could be one critical section. Could you perhaps merge these while at it? Could be a follow-up patch. Rest of the patch looks fine to me. Reviewed-by: Jiri Pirko > > txq_stats = &priv->xstats.txq_stats[chan]; >- flags = u64_stats_update_begin_irqsave(&txq_stats->syncp); >+ spin_lock(&txq_stats->lock); >+ u64_stats_update_begin(&txq_stats->syncp); > txq_stats->napi_poll++; >- u64_stats_update_end_irqrestore(&txq_stats->syncp, flags); >+ u64_stats_update_end(&txq_stats->syncp); >+ spin_unlock_irqrestore(&txq_stats->lock, flags); > > tx_done = stmmac_tx_clean(priv, budget, chan, &tx_pending_packets); > tx_done = min(tx_done, budget); >@@ -7371,10 +7395,14 @@ int stmmac_dvr_probe(struct device *device, > priv->device = device; > priv->dev = ndev; > >- for (i = 0; i < MTL_MAX_RX_QUEUES; i++) >+ for (i = 0; i < MTL_MAX_RX_QUEUES; i++) { > u64_stats_init(&priv->xstats.rxq_stats[i].syncp); >- for (i = 0; i < MTL_MAX_TX_QUEUES; i++) >+ spin_lock_init(&priv->xstats.rxq_stats[i].lock); >+ } >+ for (i = 0; i < MTL_MAX_TX_QUEUES; i++) { > u64_stats_init(&priv->xstats.txq_stats[i].syncp); >+ spin_lock_init(&priv->xstats.txq_stats[i].lock); >+ } > > stmmac_set_ethtool_ops(ndev); > priv->pause = pause; >-- >2.43.0 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel