On Wed, 2019-10-16 at 15:50 -0700, Jonathan Lemon wrote:
> From: Tariq Toukan <tar...@mellanox.com>
>
> Obsolete the RX page-cache layer, pages are now recycled
> in page_pool.
>
> This patch introduce a temporary degradation as recycling
> does not keep the pages DMA-mapped. That is fixed in a
> downstream patch.
Tariq,
i think we need to have performance numbers here to show degradation.
i am sure that non XDP traffic TCP/UDP performance will be hit.
>
> Issue: 1487631
> Signed-off-by: Tariq Toukan <tar...@mellanox.com>
>
> Signed-off-by: Jonathan Lemon <jonathan.le...@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 13 ----
> .../net/ethernet/mellanox/mlx5/core/en_main.c | 16 -----
> .../net/ethernet/mellanox/mlx5/core/en_rx.c | 67 ++---------------
> --
> 3 files changed, 4 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index 8d76452cacdc..0595cdcff594 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -583,18 +583,6 @@ struct mlx5e_mpw_info {
>
> #define MLX5E_MAX_RX_FRAGS 4
>
> -/* a single cache unit is capable to serve one napi call (for non-
> striding rq)
> - * or a MPWQE (for striding rq).
> - */
> -#define MLX5E_CACHE_UNIT (MLX5_MPWRQ_PAGES_PER_WQE >
> NAPI_POLL_WEIGHT ? \
> - MLX5_MPWRQ_PAGES_PER_WQE :
> NAPI_POLL_WEIGHT)
> -#define MLX5E_CACHE_SIZE (4 *
> roundup_pow_of_two(MLX5E_CACHE_UNIT))
> -struct mlx5e_page_cache {
> - u32 head;
> - u32 tail;
> - struct mlx5e_dma_info page_cache[MLX5E_CACHE_SIZE];
> -};
> -
> struct mlx5e_rq;
> typedef void (*mlx5e_fp_handle_rx_cqe)(struct mlx5e_rq*, struct
> mlx5_cqe64*);
> typedef struct sk_buff *
> @@ -658,7 +646,6 @@ struct mlx5e_rq {
> struct mlx5e_rq_stats *stats;
> struct mlx5e_cq cq;
> struct mlx5e_cq_decomp cqd;
> - struct mlx5e_page_cache page_cache;
> struct hwtstamp_config *tstamp;
> struct mlx5_clock *clock;
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 7569287f8f3c..168be1f800a3 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -612,9 +612,6 @@ static int mlx5e_alloc_rq(struct mlx5e_channel
> *c,
> rq->dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
> }
>
> - rq->page_cache.head = 0;
> - rq->page_cache.tail = 0;
> -
> return 0;
>
> err_free:
> @@ -640,8 +637,6 @@ static int mlx5e_alloc_rq(struct mlx5e_channel
> *c,
>
> static void mlx5e_free_rq(struct mlx5e_rq *rq)
> {
> - int i;
> -
> if (rq->xdp_prog)
> bpf_prog_put(rq->xdp_prog);
>
> @@ -655,17 +650,6 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
> mlx5e_free_di_list(rq);
> }
>
> - for (i = rq->page_cache.head; i != rq->page_cache.tail;
> - i = (i + 1) & (MLX5E_CACHE_SIZE - 1)) {
> - struct mlx5e_dma_info *dma_info = &rq-
> >page_cache.page_cache[i];
> -
> - /* With AF_XDP, page_cache is not used, so this loop is
> not
> - * entered, and it's safe to call
> mlx5e_page_release_dynamic
> - * directly.
> - */
> - mlx5e_page_release_dynamic(rq, dma_info, false);
> - }
> -
> xdp_rxq_info_unreg(&rq->xdp_rxq);
> page_pool_destroy(rq->page_pool);
> mlx5_wq_destroy(&rq->wq_ctrl);
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index d6a547238de0..a3773f8a4931 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -184,65 +184,9 @@ static inline u32
> mlx5e_decompress_cqes_start(struct mlx5e_rq *rq,
> return mlx5e_decompress_cqes_cont(rq, wq, 1, budget_rem) - 1;
> }
>
> -static inline bool mlx5e_page_is_reserved(struct page *page)
> -{
> - return page_is_pfmemalloc(page) || page_to_nid(page) !=
> numa_mem_id();
> -}
> -
> -static inline bool mlx5e_rx_cache_put(struct mlx5e_rq *rq,
> - struct mlx5e_dma_info *dma_info)
> -{
> - struct mlx5e_page_cache *cache = &rq->page_cache;
> - u32 tail_next = (cache->tail + 1) & (MLX5E_CACHE_SIZE - 1);
> - struct mlx5e_rq_stats *stats = rq->stats;
> -
> - if (tail_next == cache->head) {
> - stats->cache_full++;
> - return false;
> - }
> -
> - if (unlikely(mlx5e_page_is_reserved(dma_info->page))) {
> - stats->cache_waive++;
> - return false;
> - }
> -
> - cache->page_cache[cache->tail] = *dma_info;
> - cache->tail = tail_next;
> - return true;
> -}
> -
> -static inline bool mlx5e_rx_cache_get(struct mlx5e_rq *rq,
> - struct mlx5e_dma_info *dma_info)
> -{
> - struct mlx5e_page_cache *cache = &rq->page_cache;
> - struct mlx5e_rq_stats *stats = rq->stats;
> -
> - if (unlikely(cache->head == cache->tail)) {
> - stats->cache_empty++;
> - return false;
> - }
> -
> - if (page_ref_count(cache->page_cache[cache->head].page) != 1) {
> - stats->cache_busy++;
> - return false;
> - }
> -
> - *dma_info = cache->page_cache[cache->head];
> - cache->head = (cache->head + 1) & (MLX5E_CACHE_SIZE - 1);
> - stats->cache_reuse++;
> -
> - dma_sync_single_for_device(rq->pdev, dma_info->addr,
> - PAGE_SIZE,
> - DMA_FROM_DEVICE);
> - return true;
> -}
> -
> static inline int mlx5e_page_alloc_pool(struct mlx5e_rq *rq,
> struct mlx5e_dma_info
> *dma_info)
> {
> - if (mlx5e_rx_cache_get(rq, dma_info))
> - return 0;
> -
> dma_info->page = page_pool_dev_alloc_pages(rq->page_pool);
> if (unlikely(!dma_info->page))
> return -ENOMEM;
> @@ -276,14 +220,11 @@ void mlx5e_page_release_dynamic(struct mlx5e_rq
> *rq,
> struct mlx5e_dma_info *dma_info,
> bool recycle)
> {
> - if (likely(recycle)) {
> - if (mlx5e_rx_cache_put(rq, dma_info))
> - return;
> + mlx5e_page_dma_unmap(rq, dma_info);
>
> - mlx5e_page_dma_unmap(rq, dma_info);
> + if (likely(recycle)) {
> page_pool_recycle_direct(rq->page_pool, dma_info-
> >page);
> } else {
> - mlx5e_page_dma_unmap(rq, dma_info);
> page_pool_release_page(rq->page_pool, dma_info->page);
> put_page(dma_info->page);
> }
> @@ -1167,7 +1108,7 @@ void mlx5e_handle_rx_cqe(struct mlx5e_rq *rq,
> struct mlx5_cqe64 *cqe)
> if (!skb) {
> /* probably for XDP */
> if (__test_and_clear_bit(MLX5E_RQ_FLAG_XDP_XMIT, rq-
> >flags)) {
> - /* do not return page to cache,
> + /* do not return page to pool,
> * it will be returned on XDP_TX completion.
> */
> goto wq_cyc_pop;
> @@ -1210,7 +1151,7 @@ void mlx5e_handle_rx_cqe_rep(struct mlx5e_rq
> *rq, struct mlx5_cqe64 *cqe)
> if (!skb) {
> /* probably for XDP */
> if (__test_and_clear_bit(MLX5E_RQ_FLAG_XDP_XMIT, rq-
> >flags)) {
> - /* do not return page to cache,
> + /* do not return page to pool,
> * it will be returned on XDP_TX completion.
> */
> goto wq_cyc_pop;
