From patchwork Wed Apr 19 23:35:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 752563 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3w7dfT3nbZz9s75 for ; Thu, 20 Apr 2017 09:35:45 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="T4aSvc4Q"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S967263AbdDSXfm (ORCPT ); Wed, 19 Apr 2017 19:35:42 -0400 Received: from mail-io0-f195.google.com ([209.85.223.195]:35214 "EHLO mail-io0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S967248AbdDSXfk (ORCPT ); Wed, 19 Apr 2017 19:35:40 -0400 Received: by mail-io0-f195.google.com with SMTP id d203so8888543iof.2 for ; Wed, 19 Apr 2017 16:35:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=+/2IZ5mkuNQTk+iaW7j2CXpZqw+RBPxn24y2/iHZFrg=; b=T4aSvc4QyOem6ykEQR+SSpCYOjvgAnWwsy8s4pEKhkm/6FIqjLwJ99w8VYm9j/4Zxl zA7cViANlb/79WsB60u6I9jvod5OZ4YqVN6lmaCphHL5EWTwdV5vUpZ09Kdbj4bxj1KU 0fGG0ITCbKZ4kSeuZ0iSQd+swdMNC4eN4huCddDG4g6/Iot3WNuEqLBmQXwTyqN+3DJF rwxIq7m2NWAhaHj2uJS5PMS0CKqzuPWpSKoJ+ADfNpT4YunCWWut8oMeQnP+i16PzUHO Oxa3zad3apXGKsx5Ist449wcmqJFlTfhaepa7QdDSIMzN/oJYKGB80keEZ5xFtErHxL6 U97g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=+/2IZ5mkuNQTk+iaW7j2CXpZqw+RBPxn24y2/iHZFrg=; b=J15VRKpeY25Ywz9EWK3iVr/uc524g0FxM8UKCalVUbRi+JOojDzUqEqV2iTmFA4zWG C5RTpVI1oAEsqKOlpTPy+1oEFGtH2CnohMDZXBLFBrsWBz33286zUrdlPqQyofRaN7eP UAJ4Dznwlgt2hVUu9/9y7Hna0/q2COqUOyW3kD3VtGGbPLOipCZbWY+GtBWBWYeZYoe/ MKYQxQN8S/mPKFrGgxQxyOd00Ivn/Q9VUCaeMrlSNNJ7nXxT2mDOJlJMOSSDxZAni1x4 945afDnmnYAJoajqovxbMBQzt4Q0H85QwqfltZzrR+4AINciorg1jzvGz6BEcCe1Q+Az cutQ== X-Gm-Message-State: AN3rC/5pBlbpn5prwDNk9oKNrTK5q7FhSgRsjtJYTHVM4tfDeaRmVHQz WFtfTtXtcUtp6Q== X-Received: by 10.99.107.72 with SMTP id g69mr5313552pgc.149.1492644939849; Wed, 19 Apr 2017 16:35:39 -0700 (PDT) Received: from ?IPv6:2620:0:1000:1704:403b:fef2:d2a2:24d9? ([2620:0:1000:1704:403b:fef2:d2a2:24d9]) by smtp.googlemail.com with ESMTPSA id q70sm6470612pgq.45.2017.04.19.16.35.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 19 Apr 2017 16:35:39 -0700 (PDT) Message-ID: <1492644938.22296.19.camel@edumazet-glaptop3.roam.corp.google.com> Subject: Re: [RFC PATCH net] net/mlx5e: Race between mlx5e_update_stats() and getting the stats From: Eric Dumazet To: Martin KaFai Lau Cc: netdev@vger.kernel.org, Saeed Mahameed , kernel-team@fb.com Date: Wed, 19 Apr 2017 16:35:38 -0700 In-Reply-To: <20170419215338.vrbvzkceufktatoo@giedrius-mbp.dhcp.thefacebook.com> References: <20170419182936.788220-1-kafai@fb.com> <1492633478.22296.14.camel@edumazet-glaptop3.roam.corp.google.com> <20170419215338.vrbvzkceufktatoo@giedrius-mbp.dhcp.thefacebook.com> X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, 2017-04-19 at 14:53 -0700, Martin KaFai Lau wrote: > Right, a temp and a memcpy should be enough to solve our spike problem. > It may be the right fix for net. > > Agree that using a spinlock is better (likely changing state_lock > to spinlock). A quick grep shows 80 line changes. Saeed, thoughts? I was not advising replacing the mutex (maybe it is a mutex for good reason), I simply suggested to use another spinlock only for this very specific section. Something like : diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h index dc52053128bc752ccd398449330c24c0bdf8b3a1..9b2e1b79fded22d55e9409cb572308190679cfdd 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h @@ -722,6 +722,7 @@ struct mlx5e_priv { struct mlx5_core_dev *mdev; struct net_device *netdev; struct mlx5e_stats stats; + spinlock_t stats_lock; struct mlx5e_tstamp tstamp; u16 q_counter; #ifdef CONFIG_MLX5_CORE_EN_DCB diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c index a004a5a1a4c22a742ef3f9939769c6b5c9445f46..b4b7d43bf899cadca2c2a17151d35acac9773859 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c @@ -315,9 +315,11 @@ static void mlx5e_get_ethtool_stats(struct net_device *dev, mlx5e_update_stats(priv); mutex_unlock(&priv->state_lock); + spin_lock(&priv->stats_lock); for (i = 0; i < NUM_SW_COUNTERS; i++) data[idx++] = MLX5E_READ_CTR64_CPU(&priv->stats.sw, sw_stats_desc, i); + spin_unlock(&priv->stats_lock); for (i = 0; i < MLX5E_NUM_Q_CNTRS(priv); i++) data[idx++] = MLX5E_READ_CTR32_CPU(&priv->stats.qcnt, diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c index 66c133757a5ee8daae122e93322306b1c5c44336..4d6672045b1126a8bab4d6f2035e6a9b830560d2 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c @@ -174,7 +174,7 @@ static void mlx5e_tx_timeout_work(struct work_struct *work) static void mlx5e_update_sw_counters(struct mlx5e_priv *priv) { - struct mlx5e_sw_stats *s = &priv->stats.sw; + struct mlx5e_sw_stats temp, *s = &temp; struct mlx5e_rq_stats *rq_stats; struct mlx5e_sq_stats *sq_stats; u64 tx_offload_none = 0; @@ -229,6 +229,9 @@ static void mlx5e_update_sw_counters(struct mlx5e_priv *priv) s->link_down_events_phy = MLX5_GET(ppcnt_reg, priv->stats.pport.phy_counters, counter_set.phys_layer_cntrs.link_down_events); + spin_lock(&priv->stats_lock); + memcpy(&priv->stats.sw, s, sizeof(*s)); + spin_unlock(&priv->stats_lock); } static void mlx5e_update_vport_counters(struct mlx5e_priv *priv) @@ -2754,11 +2757,13 @@ mlx5e_get_stats(struct net_device *dev, struct rtnl_link_stats64 *stats) stats->tx_packets = PPORT_802_3_GET(pstats, a_frames_transmitted_ok); stats->tx_bytes = PPORT_802_3_GET(pstats, a_octets_transmitted_ok); } else { + spin_lock(&priv->stats_lock); stats->rx_packets = sstats->rx_packets; stats->rx_bytes = sstats->rx_bytes; stats->tx_packets = sstats->tx_packets; stats->tx_bytes = sstats->tx_bytes; stats->tx_dropped = sstats->tx_queue_dropped; + spin_unlock(&priv->stats_lock); } stats->rx_dropped = priv->stats.qcnt.rx_out_of_buffer; @@ -3561,6 +3566,8 @@ static void mlx5e_build_nic_netdev_priv(struct mlx5_core_dev *mdev, mutex_init(&priv->state_lock); + spin_lock_init(&priv->stats_lock); + INIT_WORK(&priv->update_carrier_work, mlx5e_update_carrier_work); INIT_WORK(&priv->set_rx_mode_work, mlx5e_set_rx_mode_work); INIT_WORK(&priv->tx_timeout_work, mlx5e_tx_timeout_work);