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

[Alexander Duyck]

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