Re: [PATCHv2] net: mvpp2: fix dma unmapping of TX buffers for fragments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
Date: Wed, 21 Dec 2016 11:28:49 +0100

> Since commit 71ce391dfb784 ("net: mvpp2: enable proper per-CPU TX
> buffers unmapping"), we are not correctly DMA unmapping TX buffers for
> fragments.
> 
> Indeed, the mvpp2_txq_inc_put() function only stores in the
> txq_cpu->tx_buffs[] array the physical address of the buffer to be
> DMA-unmapped when skb != NULL. In addition, when DMA-unmapping, we use
> skb_headlen(skb) to get the size to be unmapped. Both of this works fine
> for TX descriptors that are associated directly to a SKB, but not the
> ones that are used for fragments, with a NULL pointer as skb:
> 
>  - We have a NULL physical address when calling DMA unmap
>  - skb_headlen(skb) crashes because skb is NULL
> 
> This causes random crashes when fragments are used.
> 
> To solve this problem, we need to:
> 
>  - Store the physical address of the buffer to be unmapped
>    unconditionally, regardless of whether it is tied to a SKB or not.
> 
>  - Store the length of the buffer to be unmapped, which requires a new
>    field.
> 
> Instead of adding a third array to store the length of the buffer to be
> unmapped, and as suggested by David Miller, this commit refactors the
> tx_buffs[] and tx_skb[] arrays of 'struct mvpp2_txq_pcpu' into a
> separate structure 'mvpp2_txq_pcpu_buf', to which a 'size' field is
> added. Therefore, instead of having three arrays to allocate/free, we
> have a single one, which also improve data locality, reducing the
> impact on the CPU cache.
> 
> Fixes: 71ce391dfb784 ("net: mvpp2: enable proper per-CPU TX buffers unmapping")
> Reported-by: Raphael G <raphael.glon@xxxxxxxxxxxx>
> Cc: Raphael G <raphael.glon@xxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
> ---
> Changes since v1:
>  - As requested by David Miller, move the per TX buffer fields into a
>    separate structure, 'struct mvpp2_txq_pcpu_buf', so that instead of
>    allocating three arrays, we allocate a single array.

Indeed, this looks a lot better.

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]