[PATCH net-next v3 0/3] net: hinic: fix three bugs about dev_get_stats

[PATCH net-next v3 0/3] net: hinic: fix three bugs about dev_get_stats

[Qiao Ma]

These patches fixes 3 bugs of hinic driver:
- fix bug that ethtool get wrong stats because of hinic_{txq|rxq}_clean_stats() is called
- avoid kernel hung in hinic_get_stats64() 
- fix bug that u64_stats_sync is not initialized

See every patch for more information.

Changes in v3:
- fixes a compile warning reported by kernel test robot <lkp@intel.com>

Changes in v2:
- fixes another 2 bugs. (v1 is a single patch, see: https://lore.kernel.org/all/07736c2b7019b6883076a06129e06e8f7c5f7154.1656487154.git.mqaio@linux.alibaba.com/).
- to fix extra bugs, hinic_dev.tx_stats/rx_stats is removed, so there is no need to use spinlock or semaphore now. 

Qiao Ma (3):
  net: hinic: fix bug that ethtool get wrong stats
  net: hinic: avoid kernel hung in hinic_get_stats64()
  net: hinic: fix bug that u64_stats_sync is not initialized

 drivers/net/ethernet/huawei/hinic/hinic_dev.h     |  3 --
 drivers/net/ethernet/huawei/hinic/hinic_ethtool.c |  3 ++
 drivers/net/ethernet/huawei/hinic/hinic_main.c    | 58 +++++++----------------
 3 files changed, 21 insertions(+), 43 deletions(-)

-- 
1.8.3.1

[PATCH net-next v3 1/3] net: hinic: fix bug that ethtool get wrong stats

[Qiao Ma]

Function hinic_get_stats64() will do two operations:
1. reads stats from every hinic_rxq/txq and accumulates them
2. calls hinic_rxq/txq_clean_stats() to clean every rxq/txq's stats

For hinic_get_stats64(), it could get right data, because it sums all
data to nic_dev->rx_stats/tx_stats.
But it is wrong for get_drv_queue_stats(), this function will read
hinic_rxq's stats, which have been cleared to zero by hinic_get_stats64().

I have observed hinic's cleanup operation by using such command:
> watch -n 1 "cat ethtool -S eth4 | tail -40"

Result before:
     ...
     rxq7_pkts: 1
     rxq7_bytes: 90
     rxq7_errors: 0
     rxq7_csum_errors: 0
     rxq7_other_errors: 0
     ...
     rxq9_pkts: 11
     rxq9_bytes: 726
     rxq9_errors: 0
     rxq9_csum_errors: 0
     rxq9_other_errors: 0
     ...
     rxq11_pkts: 0
     rxq11_bytes: 0
     rxq11_errors: 0
     rxq11_csum_errors: 0
     rxq11_other_errors: 0

Result after a few seconds:
     ...
     rxq7_pkts: 0
     rxq7_bytes: 0
     rxq7_errors: 0
     rxq7_csum_errors: 0
     rxq7_other_errors: 0
     ...
     rxq9_pkts: 2
     rxq9_bytes: 132
     rxq9_errors: 0
     rxq9_csum_errors: 0
     rxq9_other_errors: 0
     ...
     rxq11_pkts: 1
     rxq11_bytes: 170
     rxq11_errors: 0
     rxq11_csum_errors: 0
     rxq11_other_errors: 0

To solve this problem, we just keep every queue's total stats in their own
queue (aka hinic_{rxq|txq}), and simply sum all per-queue stats every time
calling hinic_get_stats64().
With that solution, there is no need to clean per-queue stats now,
and there is no need to maintain global hinic_dev.{tx|rx}_stats, too.

Fixes: edd384f682cc ("net-next/hinic: Add ethtool and stats")
Signed-off-by: Qiao Ma <mqaio@linux.alibaba.com>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_dev.h  |  3 --
 drivers/net/ethernet/huawei/hinic/hinic_main.c | 56 +++++++++-----------------
 2 files changed, 18 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_dev.h b/drivers/net/ethernet/huawei/hinic/hinic_dev.h
index fb3e89141a0d..a4fbf44f944c 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_dev.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_dev.h
@@ -95,9 +95,6 @@ struct hinic_dev {
 	u16				sq_depth;
 	u16				rq_depth;
 
-	struct hinic_txq_stats          tx_stats;
-	struct hinic_rxq_stats          rx_stats;
-
 	u8				rss_tmpl_idx;
 	u8				rss_hash_engine;
 	u16				num_rss;
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_main.c b/drivers/net/ethernet/huawei/hinic/hinic_main.c
index 56a89793f47d..0fc048a7b244 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_main.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_main.c
@@ -80,56 +80,48 @@ static int set_features(struct hinic_dev *nic_dev,
 			netdev_features_t pre_features,
 			netdev_features_t features, bool force_change);
 
-static void update_rx_stats(struct hinic_dev *nic_dev, struct hinic_rxq *rxq)
+static void gather_rx_stats(struct hinic_rxq_stats *nic_rx_stats, struct hinic_rxq *rxq)
 {
-	struct hinic_rxq_stats *nic_rx_stats = &nic_dev->rx_stats;
 	struct hinic_rxq_stats rx_stats;
 
 	u64_stats_init(&rx_stats.syncp);
 
 	hinic_rxq_get_stats(rxq, &rx_stats);
 
-	u64_stats_update_begin(&nic_rx_stats->syncp);
 	nic_rx_stats->bytes += rx_stats.bytes;
 	nic_rx_stats->pkts  += rx_stats.pkts;
 	nic_rx_stats->errors += rx_stats.errors;
 	nic_rx_stats->csum_errors += rx_stats.csum_errors;
 	nic_rx_stats->other_errors += rx_stats.other_errors;
-	u64_stats_update_end(&nic_rx_stats->syncp);
-
-	hinic_rxq_clean_stats(rxq);
 }
 
-static void update_tx_stats(struct hinic_dev *nic_dev, struct hinic_txq *txq)
+static void gather_tx_stats(struct hinic_txq_stats *nic_tx_stats, struct hinic_txq *txq)
 {
-	struct hinic_txq_stats *nic_tx_stats = &nic_dev->tx_stats;
 	struct hinic_txq_stats tx_stats;
 
 	u64_stats_init(&tx_stats.syncp);
 
 	hinic_txq_get_stats(txq, &tx_stats);
 
-	u64_stats_update_begin(&nic_tx_stats->syncp);
 	nic_tx_stats->bytes += tx_stats.bytes;
 	nic_tx_stats->pkts += tx_stats.pkts;
 	nic_tx_stats->tx_busy += tx_stats.tx_busy;
 	nic_tx_stats->tx_wake += tx_stats.tx_wake;
 	nic_tx_stats->tx_dropped += tx_stats.tx_dropped;
 	nic_tx_stats->big_frags_pkts += tx_stats.big_frags_pkts;
-	u64_stats_update_end(&nic_tx_stats->syncp);
-
-	hinic_txq_clean_stats(txq);
 }
 
-static void update_nic_stats(struct hinic_dev *nic_dev)
+static void gather_nic_stats(struct hinic_dev *nic_dev,
+			   struct hinic_rxq_stats *nic_rx_stats,
+			   struct hinic_txq_stats *nic_tx_stats)
 {
 	int i, num_qps = hinic_hwdev_num_qps(nic_dev->hwdev);
 
 	for (i = 0; i < num_qps; i++)
-		update_rx_stats(nic_dev, &nic_dev->rxqs[i]);
+		gather_rx_stats(nic_rx_stats, &nic_dev->rxqs[i]);
 
 	for (i = 0; i < num_qps; i++)
-		update_tx_stats(nic_dev, &nic_dev->txqs[i]);
+		gather_tx_stats(nic_tx_stats, &nic_dev->txqs[i]);
 }
 
 /**
@@ -558,8 +550,6 @@ int hinic_close(struct net_device *netdev)
 	netif_carrier_off(netdev);
 	netif_tx_disable(netdev);
 
-	update_nic_stats(nic_dev);
-
 	up(&nic_dev->mgmt_lock);
 
 	if (!HINIC_IS_VF(nic_dev->hwdev->hwif))
@@ -853,26 +843,24 @@ static void hinic_get_stats64(struct net_device *netdev,
 			      struct rtnl_link_stats64 *stats)
 {
 	struct hinic_dev *nic_dev = netdev_priv(netdev);
-	struct hinic_rxq_stats *nic_rx_stats;
-	struct hinic_txq_stats *nic_tx_stats;
+	struct hinic_rxq_stats nic_rx_stats = {};
+	struct hinic_txq_stats nic_tx_stats = {};
 
-	nic_rx_stats = &nic_dev->rx_stats;
-	nic_tx_stats = &nic_dev->tx_stats;
+	u64_stats_init(&nic_rx_stats.syncp);
+	u64_stats_init(&nic_tx_stats.syncp);
 
 	down(&nic_dev->mgmt_lock);
-
 	if (nic_dev->flags & HINIC_INTF_UP)
-		update_nic_stats(nic_dev);
-
+		gather_nic_stats(nic_dev, &nic_rx_stats, &nic_tx_stats);
 	up(&nic_dev->mgmt_lock);
 
-	stats->rx_bytes   = nic_rx_stats->bytes;
-	stats->rx_packets = nic_rx_stats->pkts;
-	stats->rx_errors  = nic_rx_stats->errors;
+	stats->rx_bytes   = nic_rx_stats.bytes;
+	stats->rx_packets = nic_rx_stats.pkts;
+	stats->rx_errors  = nic_rx_stats.errors;
 
-	stats->tx_bytes   = nic_tx_stats->bytes;
-	stats->tx_packets = nic_tx_stats->pkts;
-	stats->tx_errors  = nic_tx_stats->tx_dropped;
+	stats->tx_bytes   = nic_tx_stats.bytes;
+	stats->tx_packets = nic_tx_stats.pkts;
+	stats->tx_errors  = nic_tx_stats.tx_dropped;
 }
 
 static int hinic_set_features(struct net_device *netdev,
@@ -1171,8 +1159,6 @@ static void hinic_free_intr_coalesce(struct hinic_dev *nic_dev)
 static int nic_dev_init(struct pci_dev *pdev)
 {
 	struct hinic_rx_mode_work *rx_mode_work;
-	struct hinic_txq_stats *tx_stats;
-	struct hinic_rxq_stats *rx_stats;
 	struct hinic_dev *nic_dev;
 	struct net_device *netdev;
 	struct hinic_hwdev *hwdev;
@@ -1234,12 +1220,6 @@ static int nic_dev_init(struct pci_dev *pdev)
 
 	sema_init(&nic_dev->mgmt_lock, 1);
 
-	tx_stats = &nic_dev->tx_stats;
-	rx_stats = &nic_dev->rx_stats;
-
-	u64_stats_init(&tx_stats->syncp);
-	u64_stats_init(&rx_stats->syncp);
-
 	nic_dev->vlan_bitmap = devm_bitmap_zalloc(&pdev->dev, VLAN_N_VID,
 						  GFP_KERNEL);
 	if (!nic_dev->vlan_bitmap) {
-- 
1.8.3.1

[PATCH net-next v3 2/3] net: hinic: avoid kernel hung in hinic_get_stats64()

[Qiao Ma]

When using hinic device as a bond slave device, and reading device stats
of master bond device, the kernel may hung.

The kernel panic calltrace as follows:
Kernel panic - not syncing: softlockup: hung tasks
Call trace:
  native_queued_spin_lock_slowpath+0x1ec/0x31c
  dev_get_stats+0x60/0xcc
  dev_seq_printf_stats+0x40/0x120
  dev_seq_show+0x1c/0x40
  seq_read_iter+0x3c8/0x4dc
  seq_read+0xe0/0x130
  proc_reg_read+0xa8/0xe0
  vfs_read+0xb0/0x1d4
  ksys_read+0x70/0xfc
  __arm64_sys_read+0x20/0x30
  el0_svc_common+0x88/0x234
  do_el0_svc+0x2c/0x90
  el0_svc+0x1c/0x30
  el0_sync_handler+0xa8/0xb0
  el0_sync+0x148/0x180

And the calltrace of task that actually caused kernel hungs as follows:
  __switch_to+124
  __schedule+548
  schedule+72
  schedule_timeout+348
  __down_common+188
  __down+24
  down+104
  hinic_get_stats64+44 [hinic]
  dev_get_stats+92
  bond_get_stats+172 [bonding]
  dev_get_stats+92
  dev_seq_printf_stats+60
  dev_seq_show+24
  seq_read_iter+964
  seq_read+220
  proc_reg_read+164
  vfs_read+172
  ksys_read+108
  __arm64_sys_read+28
  el0_svc_common+132
  do_el0_svc+40
  el0_svc+24
  el0_sync_handler+164
  el0_sync+324

When getting device stats from bond, kernel will call bond_get_stats().
It first holds the spinlock bond->stats_lock, and then call
hinic_get_stats64() to collect hinic device's stats.
However, hinic_get_stats64() calls `down(&nic_dev->mgmt_lock)` to
protect its critical section, which may schedule current task out.
And if system is under high pressure, the task cannot be woken up
immediately, which eventually triggers kernel hung panic.

Since previous patch has replaced hinic_dev.tx_stats/rx_stats with local
variable in hinic_get_stats64(), there is nothing need to be protected
by lock, so just removing down()/up() is ok.

Fixes: edd384f682cc ("net-next/hinic: Add ethtool and stats")
Signed-off-by: Qiao Ma <mqaio@linux.alibaba.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_main.c b/drivers/net/ethernet/huawei/hinic/hinic_main.c
index 0fc048a7b244..57ed73653792 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_main.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_main.c
@@ -849,10 +849,8 @@ static void hinic_get_stats64(struct net_device *netdev,
 	u64_stats_init(&nic_rx_stats.syncp);
 	u64_stats_init(&nic_tx_stats.syncp);
 
-	down(&nic_dev->mgmt_lock);
 	if (nic_dev->flags & HINIC_INTF_UP)
 		gather_nic_stats(nic_dev, &nic_rx_stats, &nic_tx_stats);
-	up(&nic_dev->mgmt_lock);
 
 	stats->rx_bytes   = nic_rx_stats.bytes;
 	stats->rx_packets = nic_rx_stats.pkts;
-- 
1.8.3.1

[PATCH net-next v3 3/3] net: hinic: fix bug that u64_stats_sync is not initialized

[Qiao Ma]

In get_drv_queue_stats(), the local variable {txq|rxq}_stats
should be initialized first before calling into
hinic_{rxq|txq}_get_stats(), this patch fixes it.

Fixes: edd384f682cc ("net-next/hinic: Add ethtool and stats")
Signed-off-by: Qiao Ma <mqaio@linux.alibaba.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_ethtool.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
index 93192f58ac88..75e9711bd2ba 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
@@ -1371,6 +1371,9 @@ static void get_drv_queue_stats(struct hinic_dev *nic_dev, u64 *data)
 	u16 i = 0, j = 0, qid = 0;
 	char *p;
 
+	u64_stats_init(&txq_stats.syncp);
+	u64_stats_init(&rxq_stats.syncp);
+
 	for (qid = 0; qid < nic_dev->num_qps; qid++) {
 		if (!nic_dev->txqs)
 			break;
-- 
1.8.3.1

Re: [PATCH net-next v3 3/3] net: hinic: fix bug that u64_stats_sync is not initialized

[Eric Dumazet]

On Tue, Jul 5, 2022 at 8:26 AM Qiao Ma <mqaio@linux.alibaba.com> wrote:
>
> In get_drv_queue_stats(), the local variable {txq|rxq}_stats
> should be initialized first before calling into
> hinic_{rxq|txq}_get_stats(), this patch fixes it.
>
> Fixes: edd384f682cc ("net-next/hinic: Add ethtool and stats")
> Signed-off-by: Qiao Ma <mqaio@linux.alibaba.com>
> ---
>  drivers/net/ethernet/huawei/hinic/hinic_ethtool.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
> index 93192f58ac88..75e9711bd2ba 100644
> --- a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
> +++ b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
> @@ -1371,6 +1371,9 @@ static void get_drv_queue_stats(struct hinic_dev *nic_dev, u64 *data)
>         u16 i = 0, j = 0, qid = 0;
>         char *p;
>
> +       u64_stats_init(&txq_stats.syncp);
> +       u64_stats_init(&rxq_stats.syncp);
> +

This is wrong really.

txq_stats and rxq_stats are local variables in get_drv_queue_stats()

It makes little sense to use u64_stats infra on them, because they are
not visible to other cpus/threads in the host.

Please remove this confusion, instead of consolidating it.

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_main.c
b/drivers/net/ethernet/huawei/hinic/hinic_main.c
index 56a89793f47d4209b9e0dc3a122801d476e61381..edaac5a33458d51a3fb3e75c5fbe5bec8385f688
100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_main.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_main.c
@@ -85,8 +85,6 @@ static void update_rx_stats(struct hinic_dev
*nic_dev, struct hinic_rxq *rxq)
        struct hinic_rxq_stats *nic_rx_stats = &nic_dev->rx_stats;
        struct hinic_rxq_stats rx_stats;

-       u64_stats_init(&rx_stats.syncp);
-
        hinic_rxq_get_stats(rxq, &rx_stats);

        u64_stats_update_begin(&nic_rx_stats->syncp);
@@ -105,8 +103,6 @@ static void update_tx_stats(struct hinic_dev
*nic_dev, struct hinic_txq *txq)
        struct hinic_txq_stats *nic_tx_stats = &nic_dev->tx_stats;
        struct hinic_txq_stats tx_stats;

-       u64_stats_init(&tx_stats.syncp);
-
        hinic_txq_get_stats(txq, &tx_stats);

        u64_stats_update_begin(&nic_tx_stats->syncp);
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_rx.c
b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
index 24b7b819dbfbad1d64116ef54058ee4887d7a056..4edf4c52787051aebc512094741bda30de27e2f0
100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_rx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
@@ -66,14 +66,13 @@ void hinic_rxq_clean_stats(struct hinic_rxq *rxq)
 /**
  * hinic_rxq_get_stats - get statistics of Rx Queue
  * @rxq: Logical Rx Queue
- * @stats: return updated stats here
+ * @stats: return updated stats here (private to caller)
  **/
 void hinic_rxq_get_stats(struct hinic_rxq *rxq, struct hinic_rxq_stats *stats)
 {
-       struct hinic_rxq_stats *rxq_stats = &rxq->rxq_stats;
+       const struct hinic_rxq_stats *rxq_stats = &rxq->rxq_stats;
        unsigned int start;

-       u64_stats_update_begin(&stats->syncp);
        do {
                start = u64_stats_fetch_begin(&rxq_stats->syncp);
                stats->pkts = rxq_stats->pkts;
@@ -83,7 +82,6 @@ void hinic_rxq_get_stats(struct hinic_rxq *rxq,
struct hinic_rxq_stats *stats)
                stats->csum_errors = rxq_stats->csum_errors;
                stats->other_errors = rxq_stats->other_errors;
        } while (u64_stats_fetch_retry(&rxq_stats->syncp, start));
-       u64_stats_update_end(&stats->syncp);
 }

 /**
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
index 87408e7bb8097de6fced7f0f2d170179b3fe93a9..2d97add1107f08f088b68a823767a92cbc6bbbdf
100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
@@ -91,14 +91,13 @@ void hinic_txq_clean_stats(struct hinic_txq *txq)
 /**
  * hinic_txq_get_stats - get statistics of Tx Queue
  * @txq: Logical Tx Queue
- * @stats: return updated stats here
+ * @stats: return updated stats here (private to caller)
  **/
 void hinic_txq_get_stats(struct hinic_txq *txq, struct hinic_txq_stats *stats)
 {
-       struct hinic_txq_stats *txq_stats = &txq->txq_stats;
+       const struct hinic_txq_stats *txq_stats = &txq->txq_stats;
        unsigned int start;

-       u64_stats_update_begin(&stats->syncp);
        do {
                start = u64_stats_fetch_begin(&txq_stats->syncp);
                stats->pkts    = txq_stats->pkts;
@@ -108,7 +107,6 @@ void hinic_txq_get_stats(struct hinic_txq *txq,
struct hinic_txq_stats *stats)
                stats->tx_dropped = txq_stats->tx_dropped;
                stats->big_frags_pkts = txq_stats->big_frags_pkts;
        } while (u64_stats_fetch_retry(&txq_stats->syncp, start));
-       u64_stats_update_end(&stats->syncp);
 }

 /**

Re: [PATCH net-next v3 3/3] net: hinic: fix bug that u64_stats_sync is not initialized

[maqiao]

在 2022/7/5 下午4:53, Eric Dumazet 写道:
> On Tue, Jul 5, 2022 at 8:26 AM Qiao Ma <mqaio@linux.alibaba.com> wrote:
>>
>> In get_drv_queue_stats(), the local variable {txq|rxq}_stats
>> should be initialized first before calling into
>> hinic_{rxq|txq}_get_stats(), this patch fixes it.
>>
>> Fixes: edd384f682cc ("net-next/hinic: Add ethtool and stats")
>> Signed-off-by: Qiao Ma <mqaio@linux.alibaba.com>
>> ---
>>   drivers/net/ethernet/huawei/hinic/hinic_ethtool.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
>> index 93192f58ac88..75e9711bd2ba 100644
>> --- a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
>> +++ b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
>> @@ -1371,6 +1371,9 @@ static void get_drv_queue_stats(struct hinic_dev *nic_dev, u64 *data)
>>          u16 i = 0, j = 0, qid = 0;
>>          char *p;
>>
>> +       u64_stats_init(&txq_stats.syncp);
>> +       u64_stats_init(&rxq_stats.syncp);
>> +
> 
> This is wrong really.
> 
> txq_stats and rxq_stats are local variables in get_drv_queue_stats()
> 
> It makes little sense to use u64_stats infra on them, because they are
> not visible to other cpus/threads in the host.
> 
> Please remove this confusion, instead of consolidating it.
Thanks, I'll remove it in next version.
> 
> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_main.c
> b/drivers/net/ethernet/huawei/hinic/hinic_main.c
> index 56a89793f47d4209b9e0dc3a122801d476e61381..edaac5a33458d51a3fb3e75c5fbe5bec8385f688
> 100644
> --- a/drivers/net/ethernet/huawei/hinic/hinic_main.c
> +++ b/drivers/net/ethernet/huawei/hinic/hinic_main.c
> @@ -85,8 +85,6 @@ static void update_rx_stats(struct hinic_dev
> *nic_dev, struct hinic_rxq *rxq)
>          struct hinic_rxq_stats *nic_rx_stats = &nic_dev->rx_stats;
>          struct hinic_rxq_stats rx_stats;
> 
> -       u64_stats_init(&rx_stats.syncp);
> -
>          hinic_rxq_get_stats(rxq, &rx_stats);
> 
>          u64_stats_update_begin(&nic_rx_stats->syncp);
> @@ -105,8 +103,6 @@ static void update_tx_stats(struct hinic_dev
> *nic_dev, struct hinic_txq *txq)
>          struct hinic_txq_stats *nic_tx_stats = &nic_dev->tx_stats;
>          struct hinic_txq_stats tx_stats;
> 
> -       u64_stats_init(&tx_stats.syncp);
> -
>          hinic_txq_get_stats(txq, &tx_stats);
> 
>          u64_stats_update_begin(&nic_tx_stats->syncp);
> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_rx.c
> b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
> index 24b7b819dbfbad1d64116ef54058ee4887d7a056..4edf4c52787051aebc512094741bda30de27e2f0
> 100644
> --- a/drivers/net/ethernet/huawei/hinic/hinic_rx.c
> +++ b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
> @@ -66,14 +66,13 @@ void hinic_rxq_clean_stats(struct hinic_rxq *rxq)
>   /**
>    * hinic_rxq_get_stats - get statistics of Rx Queue
>    * @rxq: Logical Rx Queue
> - * @stats: return updated stats here
> + * @stats: return updated stats here (private to caller)
>    **/
>   void hinic_rxq_get_stats(struct hinic_rxq *rxq, struct hinic_rxq_stats *stats)
>   {
> -       struct hinic_rxq_stats *rxq_stats = &rxq->rxq_stats;
> +       const struct hinic_rxq_stats *rxq_stats = &rxq->rxq_stats;
>          unsigned int start;
> 
> -       u64_stats_update_begin(&stats->syncp);
>          do {
>                  start = u64_stats_fetch_begin(&rxq_stats->syncp);
>                  stats->pkts = rxq_stats->pkts;
> @@ -83,7 +82,6 @@ void hinic_rxq_get_stats(struct hinic_rxq *rxq,
> struct hinic_rxq_stats *stats)
>                  stats->csum_errors = rxq_stats->csum_errors;
>                  stats->other_errors = rxq_stats->other_errors;
>          } while (u64_stats_fetch_retry(&rxq_stats->syncp, start));
> -       u64_stats_update_end(&stats->syncp);
>   }
> 
>   /**
> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
> b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
> index 87408e7bb8097de6fced7f0f2d170179b3fe93a9..2d97add1107f08f088b68a823767a92cbc6bbbdf
> 100644
> --- a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
> +++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
> @@ -91,14 +91,13 @@ void hinic_txq_clean_stats(struct hinic_txq *txq)
>   /**
>    * hinic_txq_get_stats - get statistics of Tx Queue
>    * @txq: Logical Tx Queue
> - * @stats: return updated stats here
> + * @stats: return updated stats here (private to caller)
>    **/
>   void hinic_txq_get_stats(struct hinic_txq *txq, struct hinic_txq_stats *stats)
>   {
> -       struct hinic_txq_stats *txq_stats = &txq->txq_stats;
> +       const struct hinic_txq_stats *txq_stats = &txq->txq_stats;
>          unsigned int start;
> 
> -       u64_stats_update_begin(&stats->syncp);
>          do {
>                  start = u64_stats_fetch_begin(&txq_stats->syncp);
>                  stats->pkts    = txq_stats->pkts;
> @@ -108,7 +107,6 @@ void hinic_txq_get_stats(struct hinic_txq *txq,
> struct hinic_txq_stats *stats)
>                  stats->tx_dropped = txq_stats->tx_dropped;
>                  stats->big_frags_pkts = txq_stats->big_frags_pkts;
>          } while (u64_stats_fetch_retry(&txq_stats->syncp, start));
> -       u64_stats_update_end(&stats->syncp);
>   }
> 
>   /**