[PATCH][net-next] net/mlx5e: avoid to call net_dim and dim_update_sample

[PATCH][net-next] net/mlx5e: avoid to call net_dim and dim_update_sample

[Li RongQing]

High cpu usage for net_dim is seen still after commit 61bf0009a765
("dim: pass dim_sample to net_dim() by reference"), the calling
net_dim can be avoid under network low throughput or pingpong mode by
checking the event counter, even under high throughput, it maybe only
rx or tx direction

And don't initialize dim_sample variable, since it will gets
overwritten by dim_update_sample

Signed-off-by: Li RongQing <lirongqing@baidu.com>
Signed-off-by: Shuo Li <lishuo02@baidu.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
index 7610829..7c525e9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
@@ -49,11 +49,19 @@ static inline bool mlx5e_channel_no_affinity_change(struct mlx5e_channel *c)
 static void mlx5e_handle_tx_dim(struct mlx5e_txqsq *sq)
 {
 	struct mlx5e_sq_stats *stats = sq->stats;
-	struct dim_sample dim_sample = {};
+	struct dim_sample dim_sample;
+	u16 nevents;
 
 	if (unlikely(!test_bit(MLX5E_SQ_STATE_DIM, &sq->state)))
 		return;
 
+	if (sq->dim->state == DIM_MEASURE_IN_PROGRESS) {
+		nevents = BIT_GAP(BITS_PER_TYPE(u16), sq->cq.event_ctr,
+						  sq->dim->start_sample.event_ctr);
+		if (nevents < DIM_NEVENTS)
+			return;
+	}
+
 	dim_update_sample(sq->cq.event_ctr, stats->packets, stats->bytes, &dim_sample);
 	net_dim(sq->dim, &dim_sample);
 }
@@ -61,11 +69,19 @@ static void mlx5e_handle_tx_dim(struct mlx5e_txqsq *sq)
 static void mlx5e_handle_rx_dim(struct mlx5e_rq *rq)
 {
 	struct mlx5e_rq_stats *stats = rq->stats;
-	struct dim_sample dim_sample = {};
+	struct dim_sample dim_sample;
+	u16 nevents;
 
 	if (unlikely(!test_bit(MLX5E_RQ_STATE_DIM, &rq->state)))
 		return;
 
+	if (rq->dim->state == DIM_MEASURE_IN_PROGRESS) {
+		nevents = BIT_GAP(BITS_PER_TYPE(u16), rq->cq.event_ctr,
+						  rq->dim->start_sample.event_ctr);
+		if (nevents < DIM_NEVENTS)
+			return;
+	}
+
 	dim_update_sample(rq->cq.event_ctr, stats->packets, stats->bytes, &dim_sample);
 	net_dim(rq->dim, &dim_sample);
 }
-- 
2.9.4

Re: [PATCH][net-next] net/mlx5e: avoid to call net_dim and dim_update_sample

[Przemek Kitszel]

On 12/12/24 12:47, Li RongQing wrote:
> High cpu usage for net_dim is seen still after commit 61bf0009a765

CPU

> ("dim: pass dim_sample to net_dim() by reference"), the calling
> net_dim can be avoid under network low throughput or pingpong mode by
> checking the event counter, even under high throughput, it maybe only

s/maybe/may be/

> rx or tx direction

Rx, Tx

> 
> And don't initialize dim_sample variable, since it will gets
> overwritten by dim_update_sample

Good point.

> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> Signed-off-by: Shuo Li <lishuo02@baidu.com>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
> index 7610829..7c525e9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
> @@ -49,11 +49,19 @@ static inline bool mlx5e_channel_no_affinity_change(struct mlx5e_channel *c)
>   static void mlx5e_handle_tx_dim(struct mlx5e_txqsq *sq)
>   {
>   	struct mlx5e_sq_stats *stats = sq->stats;
> -	struct dim_sample dim_sample = {};
> +	struct dim_sample dim_sample;
> +	u16 nevents;
>   
>   	if (unlikely(!test_bit(MLX5E_SQ_STATE_DIM, &sq->state)))
>   		return;
>   
> +	if (sq->dim->state == DIM_MEASURE_IN_PROGRESS) {
> +		nevents = BIT_GAP(BITS_PER_TYPE(u16), sq->cq.event_ctr,
> +						  sq->dim->start_sample.event_ctr);

indentation is off

> +		if (nevents < DIM_NEVENTS)
> +			return;

this very piece of code is present in net_dim()...

> +	}
> +
>   	dim_update_sample(sq->cq.event_ctr, stats->packets, stats->bytes, &dim_sample);
>   	net_dim(sq->dim, &dim_sample);
>   }
> @@ -61,11 +69,19 @@ static void mlx5e_handle_tx_dim(struct mlx5e_txqsq *sq)
>   static void mlx5e_handle_rx_dim(struct mlx5e_rq *rq)
>   {
>   	struct mlx5e_rq_stats *stats = rq->stats;
> -	struct dim_sample dim_sample = {};
> +	struct dim_sample dim_sample;
> +	u16 nevents;
>   
>   	if (unlikely(!test_bit(MLX5E_RQ_STATE_DIM, &rq->state)))
>   		return;
>   
> +	if (rq->dim->state == DIM_MEASURE_IN_PROGRESS) {
> +		nevents = BIT_GAP(BITS_PER_TYPE(u16), rq->cq.event_ctr,
> +						  rq->dim->start_sample.event_ctr);
> +		if (nevents < DIM_NEVENTS)
> +			return;
> +	}

and you have copied it twice only to avoid ktime_get() call?

> +
>   	dim_update_sample(rq->cq.event_ctr, stats->packets, stats->bytes, &dim_sample);
>   	net_dim(rq->dim, &dim_sample);
>   }

Re: [PATCH][net-next] net/mlx5e: avoid to call net_dim and dim_update_sample

[Vadim Fedorenko]

On 12/12/2024 11:47, Li RongQing wrote:
> High cpu usage for net_dim is seen still after commit 61bf0009a765
> ("dim: pass dim_sample to net_dim() by reference"), the calling
> net_dim can be avoid under network low throughput or pingpong mode by
> checking the event counter, even under high throughput, it maybe only
> rx or tx direction
> 
> And don't initialize dim_sample variable, since it will gets
> overwritten by dim_update_sample

dim_update_sample doesn't init dim_sample::comp_ctr, which is later used 
in net_dim()->dim_calc_stats(). This change can bring uninitialized
memory to the whole calculation. Keep it initialized.

> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> Signed-off-by: Shuo Li <lishuo02@baidu.com>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
> index 7610829..7c525e9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
> @@ -49,11 +49,19 @@ static inline bool mlx5e_channel_no_affinity_change(struct mlx5e_channel *c)
>   static void mlx5e_handle_tx_dim(struct mlx5e_txqsq *sq)
>   {
>   	struct mlx5e_sq_stats *stats = sq->stats;
> -	struct dim_sample dim_sample = {};
> +	struct dim_sample dim_sample;
> +	u16 nevents;
>   
>   	if (unlikely(!test_bit(MLX5E_SQ_STATE_DIM, &sq->state)))
>   		return;
>   
> +	if (sq->dim->state == DIM_MEASURE_IN_PROGRESS) {
> +		nevents = BIT_GAP(BITS_PER_TYPE(u16), sq->cq.event_ctr,
> +						  sq->dim->start_sample.event_ctr);
> +		if (nevents < DIM_NEVENTS)
> +			return;
> +	}
> +
>   	dim_update_sample(sq->cq.event_ctr, stats->packets, stats->bytes, &dim_sample);
>   	net_dim(sq->dim, &dim_sample);
>   }
> @@ -61,11 +69,19 @@ static void mlx5e_handle_tx_dim(struct mlx5e_txqsq *sq)
>   static void mlx5e_handle_rx_dim(struct mlx5e_rq *rq)
>   {
>   	struct mlx5e_rq_stats *stats = rq->stats;
> -	struct dim_sample dim_sample = {};
> +	struct dim_sample dim_sample;
> +	u16 nevents;
>   
>   	if (unlikely(!test_bit(MLX5E_RQ_STATE_DIM, &rq->state)))
>   		return;
>   
> +	if (rq->dim->state == DIM_MEASURE_IN_PROGRESS) {
> +		nevents = BIT_GAP(BITS_PER_TYPE(u16), rq->cq.event_ctr,
> +						  rq->dim->start_sample.event_ctr);
> +		if (nevents < DIM_NEVENTS)
> +			return;
> +	}
> +
>   	dim_update_sample(rq->cq.event_ctr, stats->packets, stats->bytes, &dim_sample);
>   	net_dim(rq->dim, &dim_sample);
>   }

答复: [外部邮件] Re: [PATCH][net-next] net/mlx5e: avoid to call net_dim and dim_update_sample

[Li,Rongqing]

> On 12/12/2024 11:47, Li RongQing wrote:
> > High cpu usage for net_dim is seen still after commit 61bf0009a765
> > ("dim: pass dim_sample to net_dim() by reference"), the calling
> > net_dim can be avoid under network low throughput or pingpong mode by
> > checking the event counter, even under high throughput, it maybe only
> > rx or tx direction
> >
> > And don't initialize dim_sample variable, since it will gets
> > overwritten by dim_update_sample
> 
> dim_update_sample doesn't init dim_sample::comp_ctr, which is later used in
> net_dim()->dim_calc_stats(). This change can bring uninitialized memory to the
> whole calculation. Keep it initialized.
> 

I see that comp_ctr is only for RDMA dim, so it is best to move the comp_ctr calculation to a RDMA dim related function.

To this patch, move the initialization before dim_update_sample() to avoid unnecessary initialization

Thanks

-Li