| Message ID | 20241212114723.38844-1-lirongqing@baidu.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net/mlx5e: avoid to call net_dim and dim_update_sample | expand |
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); > }
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); > }
> 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
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); }
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(-)