lkml.org 
[lkml]   [2016]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [net-next PATCH RFC 02/26] swiotlb: Add support for DMA_ATTR_SKIP_CPU_SYNC
On Mon, Oct 24, 2016 at 11:09 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Mon, Oct 24, 2016 at 08:04:37AM -0400, Alexander Duyck wrote:
>> As a first step to making DMA_ATTR_SKIP_CPU_SYNC apply to architectures
>> beyond just ARM I need to make it so that the swiotlb will respect the
>> flag. In order to do that I also need to update the swiotlb-xen since it
>> heavily makes use of the functionality.
>>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> ---
>> drivers/xen/swiotlb-xen.c | 40 ++++++++++++++++++++++----------------
>> include/linux/swiotlb.h | 6 ++++--
>> lib/swiotlb.c | 48 +++++++++++++++++++++++++++------------------
>> 3 files changed, 56 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
>> index 87e6035..cf047d8 100644
>> --- a/drivers/xen/swiotlb-xen.c
>> +++ b/drivers/xen/swiotlb-xen.c
>> @@ -405,7 +405,8 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
>> */
>> trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);
>>
>> - map = swiotlb_tbl_map_single(dev, start_dma_addr, phys, size, dir);
>> + map = swiotlb_tbl_map_single(dev, start_dma_addr, phys, size, dir,
>> + attrs);
>> if (map == SWIOTLB_MAP_ERROR)
>> return DMA_ERROR_CODE;
>>
>> @@ -416,11 +417,13 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
>> /*
>> * Ensure that the address returned is DMA'ble
>> */
>> - if (!dma_capable(dev, dev_addr, size)) {
>> - swiotlb_tbl_unmap_single(dev, map, size, dir);
>> - dev_addr = 0;
>> - }
>> - return dev_addr;
>> + if (dma_capable(dev, dev_addr, size))
>> + return dev_addr;
>> +
>> + swiotlb_tbl_unmap_single(dev, map, size, dir,
>> + attrs | DMA_ATTR_SKIP_CPU_SYNC);
>> +
>> + return DMA_ERROR_CODE;
>
> Why? This change (re-ordering the code - and returning DMA_ERROR_CODE instead
> of 0) does not have anything to do with the title.
>
> If you really feel strongly about it - then please send it as a seperate patch.

Okay I can do that. This was mostly just to clean up the formatting
because I was over 80 characters when I added the attribute. Changing
the return value to DMA_ERROR_CODE from 0 was based on the fact that
earlier in the function that is the value you return if there is a
mapping error.

>> }
>> EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
>>
>> @@ -444,7 +447,7 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
>>
>> /* NOTE: We use dev_addr here, not paddr! */
>> if (is_xen_swiotlb_buffer(dev_addr)) {
>> - swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
>> + swiotlb_tbl_unmap_single(hwdev, paddr, size, dir, attrs);
>> return;
>> }
>>
>> @@ -557,16 +560,9 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
>> start_dma_addr,
>> sg_phys(sg),
>> sg->length,
>> - dir);
>> - if (map == SWIOTLB_MAP_ERROR) {
>> - dev_warn(hwdev, "swiotlb buffer is full\n");
>> - /* Don't panic here, we expect map_sg users
>> - to do proper error handling. */
>> - xen_swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
>> - attrs);
>> - sg_dma_len(sgl) = 0;
>> - return 0;
>> - }
>> + dir, attrs);
>> + if (map == SWIOTLB_MAP_ERROR)
>> + goto map_error;
>> xen_dma_map_page(hwdev, pfn_to_page(map >> PAGE_SHIFT),
>> dev_addr,
>> map & ~PAGE_MASK,
>> @@ -589,6 +585,16 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
>> sg_dma_len(sg) = sg->length;
>> }
>> return nelems;
>> +map_error:
>> + dev_warn(hwdev, "swiotlb buffer is full\n");
>> + /*
>> + * Don't panic here, we expect map_sg users
>> + * to do proper error handling.
>> + */
>> + xen_swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
>> + attrs | DMA_ATTR_SKIP_CPU_SYNC);
>> + sg_dma_len(sgl) = 0;
>> + return 0;
>> }
>
> This too. Why can't that be part of the existing code that was there?

Once again it was a formatting thing. I was indented too far and
adding the attribute pushed me over 80 characters so I broke it out to
a label to avoid the problem.

- Alex

\
 
 \ /
  Last update: 2016-10-24 21:18    [W:0.162 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site