On Mon, Jul 11, 2016 at 02:48:17PM +0300, Saeed Mahameed wrote:
[...]
> 
> yes, we need something like:
> 
> +static inline void
> +mlx4_en_sync_dma(struct mlx4_en_priv *priv,
> +                struct mlx4_en_rx_desc *rx_desc,
> +                int length)
> +{
> +       dma_addr_t dma;
> +
> +       /* Sync dma addresses from HW descriptor */
> +       for (nr = 0; nr < priv->num_frags; nr++) {
> +               struct mlx4_en_frag_info *frag_info = &priv->frag_info[nr];
> +
> +               if (length <= frag_info->frag_prefix_size)
> +                       break;
> +
> +               dma = be64_to_cpu(rx_desc->data[nr].addr);
> +               dma_sync_single_for_cpu(priv->ddev, dma, frag_info->frag_size,
> +                                       DMA_FROM_DEVICE);
> +       }
> +}
> 
> 
> @@ -790,6 +808,10 @@ int mlx4_en_process_rx_cq(struct net_device *dev,
> struct mlx4_en_cq *cq, int bud
>                         goto next;
>                 }
> 
> +               length = be32_to_cpu(cqe->byte_cnt);
> +               length -= ring->fcs_del;
> +
> +               mlx4_en_sync_dma(priv,rx_desc, length);
>                  /* data is available continue processing the packet */
> 
> and make sure to remove all explicit dma_sync_single_for_cpu calls.

I see. At first glance, this may work, but introduces some changes in
the driver that may be unwanted. For instance, the dma sync cost is now
being paid even in the case where no skb will be allocated. So, under
memory pressure, it might cause extra work which would slow down your
ability to recover from the stress.

Let's keep discussing it, but in the context of a standalone cleanup.

Reply via email to