From: "Jonathan Lemon" <jonathan.lemon@gmail.com>
To: "Jesper Dangaard Brouer" <brouer@redhat.com>
Cc: ilias.apalodimas@linaro.org, saeedm@mellanox.com,
tariqt@mellanox.com, netdev@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 09/10 net-next] net/mlx5: Add page_pool stats to the Mellanox driver
Date: Fri, 18 Oct 2019 13:45:47 -0700 [thread overview]
Message-ID: <361B12F0-625B-4148-91EC-A2217679C723@gmail.com> (raw)
In-Reply-To: <20191017130935.01a7a99b@carbon>
On 17 Oct 2019, at 4:09, Jesper Dangaard Brouer wrote:
> On Wed, 16 Oct 2019 15:50:27 -0700
> Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
>
>> Replace the now deprecated inernal cache stats with the page pool
>> stats.
>
> I can see that the stats you introduced are useful, but they have to
> be
> implemented in way that does not hurt performance.
They're not noticeable, but even if they were, they are needed
for production, otherwise there's no way to identify problems.
I can separate the ring consume/produce counters so they
are always separated by a cache line distance.
>> # ethtool -S eth0 | grep rx_pool
>> rx_pool_cache_hit: 1646798
>> rx_pool_cache_full: 0
>> rx_pool_cache_empty: 15723566
>> rx_pool_ring_produce: 474958
>> rx_pool_ring_consume: 0
>> rx_pool_ring_return: 474958
>> rx_pool_flush: 144
>> rx_pool_node_change: 0
>>
>> Showing about a 10% hit rate for the page pool.
>
> What is the workload from above stats?
Network traffic from a proxygen load balancer. From
this, we see the ptr_ring is completely unused except
for flushing operations.
--
Jonathan
>
>
>> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/en.h | 1 +
>> .../net/ethernet/mellanox/mlx5/core/en_main.c | 1 +
>> .../ethernet/mellanox/mlx5/core/en_stats.c | 39
>> ++++++++++++-------
>> .../ethernet/mellanox/mlx5/core/en_stats.h | 19 +++++----
>> 4 files changed, 35 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h
>> b/drivers/net/ethernet/mellanox/mlx5/core/en.h
>> index 2e281c755b65..b34519061d12 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
>> @@ -50,6 +50,7 @@
>> #include <net/xdp.h>
>> #include <linux/dim.h>
>> #include <linux/bits.h>
>> +#include <net/page_pool.h>
>> #include "wq.h"
>> #include "mlx5_core.h"
>> #include "en_stats.h"
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> index 2b828de1adf0..f10b5838fb17 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> @@ -551,6 +551,7 @@ static int mlx5e_alloc_rq(struct mlx5e_channel
>> *c,
>> pp_params.nid = cpu_to_node(c->cpu);
>> pp_params.dev = c->pdev;
>> pp_params.dma_dir = rq->buff.map_dir;
>> + pp_params.stats = &rq->stats->pool;
>>
>> /* page_pool can be used even when there is no rq->xdp_prog,
>> * given page_pool does not handle DMA mapping there is no
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> index ac6fdcda7019..ad42d965d786 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> @@ -102,11 +102,14 @@ static const struct counter_desc
>> sw_stats_desc[] = {
>> { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_buff_alloc_err) },
>> { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_cqe_compress_blks)
>> },
>> { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_cqe_compress_pkts)
>> },
>> - { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_cache_reuse) },
>> - { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_cache_full) },
>> - { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_cache_empty) },
>> - { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_cache_busy) },
>> - { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_cache_waive) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_pool_cache_hit) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_pool_cache_full) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_pool_cache_empty) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_pool_ring_produce)
>> },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_pool_ring_consume)
>> },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_pool_ring_return) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_pool_flush) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_pool_node_change) },
>> { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_congst_umr) },
>> { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_arfs_err) },
>> { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_recover) },
>> @@ -214,11 +217,14 @@ static void mlx5e_grp_sw_update_stats(struct
>> mlx5e_priv *priv)
>> s->rx_buff_alloc_err += rq_stats->buff_alloc_err;
>> s->rx_cqe_compress_blks += rq_stats->cqe_compress_blks;
>> s->rx_cqe_compress_pkts += rq_stats->cqe_compress_pkts;
>> - s->rx_cache_reuse += rq_stats->cache_reuse;
>> - s->rx_cache_full += rq_stats->cache_full;
>> - s->rx_cache_empty += rq_stats->cache_empty;
>> - s->rx_cache_busy += rq_stats->cache_busy;
>> - s->rx_cache_waive += rq_stats->cache_waive;
>> + s->rx_pool_cache_hit += rq_stats->pool.cache_hit;
>> + s->rx_pool_cache_full += rq_stats->pool.cache_full;
>> + s->rx_pool_cache_empty += rq_stats->pool.cache_empty;
>> + s->rx_pool_ring_produce += rq_stats->pool.ring_produce;
>> + s->rx_pool_ring_consume += rq_stats->pool.ring_consume;
>> + s->rx_pool_ring_return += rq_stats->pool.ring_return;
>> + s->rx_pool_flush += rq_stats->pool.flush;
>> + s->rx_pool_node_change += rq_stats->pool.node_change;
>> s->rx_congst_umr += rq_stats->congst_umr;
>> s->rx_arfs_err += rq_stats->arfs_err;
>> s->rx_recover += rq_stats->recover;
>> @@ -1446,11 +1452,14 @@ static const struct counter_desc
>> rq_stats_desc[] = {
>> { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, buff_alloc_err) },
>> { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, cqe_compress_blks)
>> },
>> { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, cqe_compress_pkts)
>> },
>> - { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, cache_reuse) },
>> - { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, cache_full) },
>> - { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, cache_empty) },
>> - { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, cache_busy) },
>> - { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, cache_waive) },
>> + { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, pool.cache_hit) },
>> + { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, pool.cache_full) },
>> + { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, pool.cache_empty) },
>> + { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, pool.ring_produce)
>> },
>> + { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, pool.ring_consume)
>> },
>> + { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, pool.ring_return) },
>> + { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, pool.flush) },
>> + { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, pool.node_change) },
>> { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, congst_umr) },
>> { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, arfs_err) },
>> { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, recover) },
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
>> b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
>> index 79f261bf86ac..7d6001969400 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
>> @@ -109,11 +109,14 @@ struct mlx5e_sw_stats {
>> u64 rx_buff_alloc_err;
>> u64 rx_cqe_compress_blks;
>> u64 rx_cqe_compress_pkts;
>> - u64 rx_cache_reuse;
>> - u64 rx_cache_full;
>> - u64 rx_cache_empty;
>> - u64 rx_cache_busy;
>> - u64 rx_cache_waive;
>> + u64 rx_pool_cache_hit;
>> + u64 rx_pool_cache_full;
>> + u64 rx_pool_cache_empty;
>> + u64 rx_pool_ring_produce;
>> + u64 rx_pool_ring_consume;
>> + u64 rx_pool_ring_return;
>> + u64 rx_pool_flush;
>> + u64 rx_pool_node_change;
>> u64 rx_congst_umr;
>> u64 rx_arfs_err;
>> u64 rx_recover;
>> @@ -245,14 +248,10 @@ struct mlx5e_rq_stats {
>> u64 buff_alloc_err;
>> u64 cqe_compress_blks;
>> u64 cqe_compress_pkts;
>> - u64 cache_reuse;
>> - u64 cache_full;
>> - u64 cache_empty;
>> - u64 cache_busy;
>> - u64 cache_waive;
>> u64 congst_umr;
>> u64 arfs_err;
>> u64 recover;
>> + struct page_pool_stats pool;
>> };
>>
>> struct mlx5e_sq_stats {
>
>
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
next prev parent reply other threads:[~2019-10-18 20:45 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-16 22:50 [PATCH 00/10 net-next] page_pool cleanups Jonathan Lemon
2019-10-16 22:50 ` [PATCH 01/10 net-next] net/mlx5e: RX, Remove RX page-cache Jonathan Lemon
2019-10-17 1:30 ` Jakub Kicinski
2019-10-18 20:03 ` Jonathan Lemon
2019-10-18 20:53 ` Saeed Mahameed
2019-10-19 0:10 ` Jonathan Lemon
2019-10-20 7:29 ` Tariq Toukan
2019-10-16 22:50 ` [PATCH 02/10 net-next] net/mlx5e: RX, Manage RX pages only via page pool API Jonathan Lemon
2019-10-16 22:50 ` [PATCH 03/10 net-next] net/mlx5e: RX, Internal DMA mapping in page_pool Jonathan Lemon
2019-10-17 1:33 ` Jakub Kicinski
2019-10-18 20:04 ` Jonathan Lemon
2019-10-18 21:01 ` Saeed Mahameed
2019-10-16 22:50 ` [PATCH 04/10 net-next] page_pool: Add API to update numa node and flush page caches Jonathan Lemon
2019-10-17 12:06 ` Ilias Apalodimas
2019-10-18 21:07 ` Saeed Mahameed
2019-10-18 23:38 ` Jonathan Lemon
2019-10-16 22:50 ` [PATCH 05/10 net-next] net/mlx5e: Rx, Update page pool numa node when changed Jonathan Lemon
2019-10-16 22:50 ` [PATCH 06/10 net-next] page_pool: Add page_pool_keep_page Jonathan Lemon
2019-10-16 22:50 ` [PATCH 07/10 net-next] page_pool: allow configurable linear cache size Jonathan Lemon
2019-10-17 8:51 ` Jesper Dangaard Brouer
2019-10-18 20:31 ` Jonathan Lemon
2019-10-16 22:50 ` [PATCH 08/10 net-next] page_pool: Add statistics Jonathan Lemon
2019-10-17 8:29 ` Jesper Dangaard Brouer
2019-10-18 21:29 ` Saeed Mahameed
2019-10-18 23:37 ` Jonathan Lemon
2019-10-16 22:50 ` [PATCH 09/10 net-next] net/mlx5: Add page_pool stats to the Mellanox driver Jonathan Lemon
2019-10-17 11:09 ` Jesper Dangaard Brouer
2019-10-18 20:45 ` Jonathan Lemon [this message]
2019-10-16 22:50 ` [PATCH 10/10 net-next] page_pool: Cleanup and rename page_pool functions Jonathan Lemon
2019-10-18 20:50 ` [PATCH 00/10 net-next] page_pool cleanups Saeed Mahameed
2019-10-18 21:21 ` Saeed Mahameed
2019-10-18 23:32 ` Jonathan Lemon
2019-10-21 19:08 ` Saeed Mahameed
2019-10-21 21:45 ` Jonathan Lemon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=361B12F0-625B-4148-91EC-A2217679C723@gmail.com \
--to=jonathan.lemon@gmail.com \
--cc=brouer@redhat.com \
--cc=ilias.apalodimas@linaro.org \
--cc=kernel-team@fb.com \
--cc=netdev@vger.kernel.org \
--cc=saeedm@mellanox.com \
--cc=tariqt@mellanox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.