Re: [mm PATCH v2 03/26] swiotlb: Add support for DMA_ATTR_SKIP_CPU_SYNC

From: Alexander Duyck
Date: Sat Nov 05 2016 - 19:13:45 EST


On Sat, Nov 5, 2016 at 12:39 PM, Konrad Rzeszutek Wilk
<konrad@xxxxxxxxxx> wrote:
> .. snip..
>> @@ -561,6 +565,7 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
>> * First, sync the memory before unmapping the entry
>> */
>> if (orig_addr != INVALID_PHYS_ADDR &&
>> + !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
>> ((dir == DMA_FROM_DEVICE) || (dir == DMA_BIDIRECTIONAL)))
>> swiotlb_bounce(orig_addr, tlb_addr, size, DMA_FROM_DEVICE);
>>
>> @@ -654,7 +659,8 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
>> * GFP_DMA memory; fall back on map_single(), which
>> * will grab memory from the lowest available address range.
>> */
>> - phys_addr_t paddr = map_single(hwdev, 0, size, DMA_FROM_DEVICE);
>> + phys_addr_t paddr = map_single(hwdev, 0, size,
>> + DMA_FROM_DEVICE, 0);
>> if (paddr == SWIOTLB_MAP_ERROR)
>> goto err_warn;
>>
>> @@ -669,7 +675,8 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
>>
>> /* DMA_TO_DEVICE to avoid memcpy in unmap_single */
>> swiotlb_tbl_unmap_single(hwdev, paddr,
>> - size, DMA_TO_DEVICE);
>> + size, DMA_TO_DEVICE,
>> + DMA_ATTR_SKIP_CPU_SYNC);
>
> This I believe is redundant. That is swiotlb_tbl_unmap_single only
> does an bounce if the dir is DMA_FROM_DEVICE or DMA_BIDIRECTIONAL.
>
> I added /* optional. */

You are probably right. I don't need to add the DMA_ATTR_SKIP_CPU_SYNC here.

>> goto err_warn;
>> }
>> }
>> @@ -699,7 +706,7 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
>> free_pages((unsigned long)vaddr, get_order(size));
>> else
>> /* DMA_TO_DEVICE to avoid memcpy in swiotlb_tbl_unmap_single */
>> - swiotlb_tbl_unmap_single(hwdev, paddr, size, DMA_TO_DEVICE);
>> + swiotlb_tbl_unmap_single(hwdev, paddr, size, DMA_TO_DEVICE, 0);
>
> .. but here you choose to put 0? I changed that to
> DMA_ATTR_SKIP_CPU_SYNC and expanded the comment above.
>
> Time to test the patches.

I think I had probably realized the fact that I didn't need it above
and so just used 0 here. I can clean this up and resubmit if you
want.

Do you want me to just split this patch set up so that I submit the
swiotlb patches to you and leave the rest of the patches for the mm
tree?

Thanks.

- Alex