Improve swiotlb performance by using physical addresses
Subsystems (4)
v2 cover Improve swiotlb performance by using physical addresses
While working on 10Gb/s routing performance I found a significant amount of time was being spent in the swiotlb DMA handler. Further digging found that a significant amount of this was due to virtual to physical address translation and calling the function that did it. It accounted for nearly 60% of the total swiotlb overhead.
This patch set works to resolve that by replacing the io_tlb_start and io_tlb_end virtual addresses with a physical addresses. In addition it changes the io_tlb_overflow_buffer from a virtual to a physical address. I followed through with the cleanup to the point that the only functions that really require the virtual address for the DMA buffer are the init, free, and bounce functions.
In the case of devices that are using the bounce buffers these patches should result in only a slight performance gain if any. This is due to the locking overhead required to map and unmap the buffers.
In the case of devices that are not making use of bounce buffers these patches can significantly reduce their overhead. In the case of an ixgbe routing test for example, these changes result in 7 fewer calls to __phys_addr and allow is_swiotlb_buffer to become inlined due to a reduction in the number of instructions. When running a routing throughput test using small packets I saw roughly a 6% increase in packets rates after applying these patches. This appears to match up with the CPU overhead reduction I was tracking via perf.
Before:
Results 10.0Mpps
After:
Results 10.6Mpps
Finally, I updated the parameter names for several of the core function calls as there was some ambiguity in naming. Specifically virtual address pointers were named dma_addr. When I changed these pointers to physical I instead used the name tlb_addr as this value represented a physical address in the io_tlb_start region and is less likely to be confused with a bus address.
v2:
I reviewed the changes and realized that the first patch that was dropping io_tlb_end and calculating the value didn't actually gain me much once I had gone through and translated the rest of the addresses to physical addresses. As such I have updated the patch so that it instead is converting io_tlb_end from a virtual address to a physical address. This actually helps to reduce the overhead for is_swiotlb_buffer and swiotlb_dma_supported by several instructions.
---
Alexander Duyck (7):
swiotlb: Do not export swiotlb_bounce since there are no external consumers
swiotlb: Use physical addresses instead of virtual in swiotlb_tbl_sync_single
swiotlb: Use physical addresses for swiotlb_tbl_unmap_single
swiotlb: Return physical addresses when calling swiotlb_tbl_map_single
swiotlb: Make io_tlb_overflow_buffer a physical address
swiotlb: Make io_tlb_start a physical address instead of a virtual one
swiotlb: Make io_tlb_end a physical address instead of a virtual one
drivers/xen/swiotlb-xen.c | 25 ++-- include/linux/swiotlb.h | 20 ++- lib/swiotlb.c | 269 +++++++++++++++++++++++---------------------- 3 files changed, 163 insertions(+), 151 deletions(-)
PATCH v2 1/7 swiotlb: Make io_tlb_end a physical address instead of a virtual one
This change replaces all references to the virtual address for io_tlb_end with references to the physical address io_tlb_end. The main advantage of replacing the virtual address with a physical address is that we can avoid having to do multiple translations from the virtual address to the physical one needed for testing an existing DMA address.
---
lib/swiotlb.c | 24 +++++++++++++----------- 1 files changed, 13 insertions(+), 11 deletions(-)
lib/swiotlb.c +13 -11
@@ -57,7 +57,8 @@ int swiotlb_force;* swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this * API. */ -static char *io_tlb_start, *io_tlb_end; +static char *io_tlb_start; +phys_addr_t io_tlb_end;/* * The number of IO TLB blocks (in groups of 64) between io_tlb_start and
@@ -125,14 +126,16 @@ static dma_addr_t swiotlb_virt_to_bus(struct device *hwdev,void swiotlb_print_info (void ) {unsigned long bytes = io_tlb_nslabs <<IO_TLB_SHIFT ; -phys_addr_t pstart, pend; +phys_addr_t pstart; +unsigned char *vend; pstart =virt_to_phys (io_tlb_start); - pend =virt_to_phys (io_tlb_end); + vend =phys_to_virt (io_tlb_end);printk (KERN_INFO "software IO TLB [mem %#010llx-%#010llx] (%luMB) mapped at [%p-%p]\n" , - (unsigned long long )pstart, (unsigned long long )pend -1 , - bytes >>20 , io_tlb_start, io_tlb_end -1 ); + (unsigned long long )pstart, + (unsigned long long )io_tlb_end, + bytes >>20 , io_tlb_start, vend -1 ); }void __init swiotlb_init_with_tbl (char *tlb,unsigned long nslabs,int verbose)
@@ -143,7 +146,7 @@ void __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) io_tlb_nslabs = nslabs; io_tlb_start = tlb; - io_tlb_end = io_tlb_start + bytes; + io_tlb_end =__pa (io_tlb_start) + bytes;/* * Allocate and initialize the free list array. This array is used
@@ -254,7 +257,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs) io_tlb_nslabs = nslabs; io_tlb_start = tlb; - io_tlb_end = io_tlb_start + bytes; + io_tlb_end =virt_to_phys (io_tlb_start) + bytes;memset (io_tlb_start,0 , bytes);
@@ -304,7 +307,7 @@ cleanup3: sizeof(int))); io_tlb_list =NULL ; cleanup2: - io_tlb_end =NULL ; + io_tlb_end =0 ; io_tlb_start =NULL ; io_tlb_nslabs =0 ;return -ENOMEM ;
@@ -339,8 +342,7 @@ void __init swiotlb_free(void)static int is_swiotlb_buffer (phys_addr_t paddr) { -return paddr >=virt_to_phys (io_tlb_start) && - paddr <virt_to_phys (io_tlb_end); +return paddr >=virt_to_phys (io_tlb_start) && paddr < io_tlb_end; }/*
@@ -938,6 +940,6 @@ EXPORT_SYMBOL(swiotlb_dma_mapping_error);int swiotlb_dma_supported (struct device *hwdev,u64 mask) { -return swiotlb_virt_to_bus (hwdev, io_tlb_end -1 ) <= mask; +return phys_to_dma (hwdev, io_tlb_end -1 ) <= mask; }EXPORT_SYMBOL (swiotlb_dma_supported);
Re: [PATCH v2 1/7] swiotlb: Make io_tlb_end a physical address instead of a virtual one
Hi Alexander,
Quoted 17 lines · Alexander Duyck
This change replaces all references to the virtual address for
io_tlb_endwith references to the physical addressio_tlb_end. The main advantage of replacing the virtual address with a physical address is that we can avoid having to do multiple translations from the virtual address to the physical one needed for testing an existing DMA address.Signed-off-by Alexander Duyck <[email protected]>---
lib/swiotlb.c | 24 +++++++++++++----------- 1 files changed, 13 insertions(+), 11 deletions(-)lib/swiotlb.c +0
-0
lib/swiotlb.c int swiotlb_force; · line 61
Quoted 6 lines
lib/swiotlb.c +2
-1@@ -57,7 +57,8 @@ int swiotlb_force;* swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this * API. */ -static char *io_tlb_start, *io_tlb_end; +static char *io_tlb_start; +phys_addr_t io_tlb_end;
If add io_tlb_start_phy and io_tlb_end_phy, could we get same results with less hunks?
Hillf
Re: [PATCH v2 1/7] swiotlb: Make io_tlb_end a physical address instead of a virtual one
Quoted 30 lines · Hillf Danton
Hi Alexander,
Quoted 17 lines · Alexander Duyck
This change replaces all references to the virtual address for
io_tlb_endwith references to the physical addressio_tlb_end. The main advantage of replacing the virtual address with a physical address is that we can avoid having to do multiple translations from the virtual address to the physical one needed for testing an existing DMA address.Signed-off-by Alexander Duyck <[email protected]>---
lib/swiotlb.c | 24 +++++++++++++----------- 1 files changed, 13 insertions(+), 11 deletions(-)lib/swiotlb.c +0
-0lib/swiotlb.c int swiotlb_force; · line 61
Quoted 6 lines
lib/swiotlb.c +2
-1@@ -57,7 +57,8 @@ int swiotlb_force;* swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this * API. */ -static char *io_tlb_start, *io_tlb_end; +static char *io_tlb_start; +phys_addr_t io_tlb_end;If add
io_tlb_start_phyandio_tlb_end_phy, could we get same results with less hunks?
Quoted 1 line
Hillf
What do you mean by less hunks? Are you referring to the memory space? If so, then the patches I am submitting do not impact how much space is used for the bounce buffer. The only real result of these patches is that the total code path is significantly reduced since we don't have to perform any virtual to physical translations in the hot-path.
Thanks,
Alex
Re: [PATCH v2 1/7] swiotlb: Make io_tlb_end a physical address instead of a virtual one
Quoted 31 lines · Alexander Duyck
Quoted 30 lines · Hillf Danton
Hi Alexander,
Quoted 17 lines · Alexander Duyck
This change replaces all references to the virtual address for
io_tlb_endwith references to the physical addressio_tlb_end. The main advantage of replacing the virtual address with a physical address is that we can avoid having to do multiple translations from the virtual address to the physical one needed for testing an existing DMA address.Signed-off-by Alexander Duyck <[email protected]>---
lib/swiotlb.c | 24 +++++++++++++----------- 1 files changed, 13 insertions(+), 11 deletions(-)lib/swiotlb.c +0
-0lib/swiotlb.c int swiotlb_force; · line 61
Quoted 6 lines
lib/swiotlb.c +2
-1@@ -57,7 +57,8 @@ int swiotlb_force;* swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this * API. */ -static char *io_tlb_start, *io_tlb_end; +static char *io_tlb_start; +phys_addr_t io_tlb_end;If add
io_tlb_start_phyandio_tlb_end_phy, could we get same results with less hunks?
Quoted 3 lines
Quoted 1 line
Hillf
What do you mean by less hunks? Are you referring to the memory space?
As in less patch movements.
Quoted 4 lines
If so, then the patches I am submitting do not impact how much space is used for the bounce buffer. The only real result of these patches is that the total code path is significantly reduced since we don't have to perform any virtual to physical translations in the hot-path.
No. He is referring that you can keep io_tlb_end still here. Just do the computation of the physical address in the init path (of the end). Then you don't need to do the shifting in the 'is-this-swiotlb-buffer' and can just do a simple:
if (dma_addr >= io_tlb_start && dma_addr <= io_tlb_end)
Re: [PATCH v2 1/7] swiotlb: Make io_tlb_end a physical address instead of a virtual one
Quoted 39 lines · Konrad Rzeszutek Wilk
Quoted 31 lines · Alexander Duyck
Quoted 30 lines · Hillf Danton
Hi Alexander,
Quoted 17 lines · Alexander Duyck
This change replaces all references to the virtual address for
io_tlb_endwith references to the physical addressio_tlb_end. The main advantage of replacing the virtual address with a physical address is that we can avoid having to do multiple translations from the virtual address to the physical one needed for testing an existing DMA address.Signed-off-by Alexander Duyck <[email protected]>---
lib/swiotlb.c | 24 +++++++++++++----------- 1 files changed, 13 insertions(+), 11 deletions(-)lib/swiotlb.c +0
-0lib/swiotlb.c int swiotlb_force; · line 61
Quoted 6 lines
lib/swiotlb.c +2
-1@@ -57,7 +57,8 @@ int swiotlb_force;* swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this * API. */ -static char *io_tlb_start, *io_tlb_end; +static char *io_tlb_start; +phys_addr_t io_tlb_end;If add
io_tlb_start_phyandio_tlb_end_phy, could we get same results with less hunks?Quoted 2 lines
Quoted 1 line
Hillf
What do you mean by less hunks? Are you referring to the memory space?
As in less patch movements.
Quoted 4 lines
If so, then the patches I am submitting do not impact how much space is used for the bounce buffer. The only real result of these patches is that the total code path is significantly reduced since we don't have to perform any virtual to physical translations in the hot-path.
Quoted 6 lines
No. He is referring that you can keep
io_tlb_endstill here. Just do the computation of the physical address in the init path (of the end). Then you don't need to do the shifting in the 'is-this-swiotlb-buffer' and can just do a simple:if (dma_addr >= io_tlb_start && dma_addr <= io_tlb_end)
That is how the code ends up. The v2 and v3 version of these patches leave the end value there. As this patch says I am just changing the end to be physical instead of virtual. I reviewed the code and realized that I wasn't saving anything by removing it since the overall code was larger as a result so I just converted it to a physical address. There are no users of io_tlb_end that are accessing it as a virtual value so all I did is just change it to a physical one and drop the virt_to_phys calls that were made on it. If I am not mistaken by the second patch the is_swiotlb_buffer call is literally what you have described above. Here is the snippet from the 2nd patch:
static int is_swiotlb_buffer(phys_addr_t paddr)
{
- return paddr >= virt_to_phys(io_tlb_start) && paddr < io_tlb_end;
+ return paddr >= io_tlb_start && paddr < io_tlb_end;
}
As far as the number of patches I decided to do this incrementally instead of trying to do it all at once. That way it is clearer to the reviewer what I am doing in each step and it can be more easily bisected in case I messed up somewhere. If you want fewer patches I can do that but I don't see the point in combining patches since they are all just going to result in the same total change anyway.
Thanks,
Alex
Re: [PATCH v2 1/7] swiotlb: Make io_tlb_end a physical address instead of a virtual one
Quoted 47 lines · Alexander Duyck
Quoted 39 lines · Konrad Rzeszutek Wilk
Quoted 31 lines · Alexander Duyck
Quoted 30 lines · Hillf Danton
Hi Alexander,
Quoted 17 lines · Alexander Duyck
This change replaces all references to the virtual address for
io_tlb_endwith references to the physical addressio_tlb_end. The main advantage of replacing the virtual address with a physical address is that we can avoid having to do multiple translations from the virtual address to the physical one needed for testing an existing DMA address.Signed-off-by Alexander Duyck <[email protected]>---
lib/swiotlb.c | 24 +++++++++++++----------- 1 files changed, 13 insertions(+), 11 deletions(-)lib/swiotlb.c +0
-0lib/swiotlb.c int swiotlb_force; · line 61
Quoted 6 lines
lib/swiotlb.c +2
-1@@ -57,7 +57,8 @@ int swiotlb_force;* swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this * API. */ -static char *io_tlb_start, *io_tlb_end; +static char *io_tlb_start; +phys_addr_t io_tlb_end;If add
io_tlb_start_phyandio_tlb_end_phy, could we get same results with less hunks?Quoted 2 lines
Quoted 1 line
Hillf
What do you mean by less hunks? Are you referring to the memory space?
As in less patch movements.
Quoted 4 lines
If so, then the patches I am submitting do not impact how much space is used for the bounce buffer. The only real result of these patches is that the total code path is significantly reduced since we don't have to perform any virtual to physical translations in the hot-path.
Quoted 6 lines
No. He is referring that you can keep
io_tlb_endstill here. Just do the computation of the physical address in the init path (of the end). Then you don't need to do the shifting in the 'is-this-swiotlb-buffer' and can just do a simple:if (dma_addr >= io_tlb_start && dma_addr <= io_tlb_end)
Quoted 2 lines
That is how the code ends up. The v2 and v3 version of these patches leave the end value there. As this patch says I am just changing the
Great!
Quoted 16 lines
end to be physical instead of virtual. I reviewed the code and realized that I wasn't saving anything by removing it since the overall code was larger as a result so I just converted it to a physical address. There are no users of
io_tlb_endthat are accessing it as a virtual value so all I did is just change it to a physical one and drop thevirt_to_physcalls that were made on it. If I am not mistaken by the second patch theis_swiotlb_buffercall is literally what you have described above. Here is the snippet from the 2nd patch:static int
is_swiotlb_buffer(phys_addr_t paddr)
{(quoted diff) +1
-1- return paddr >= virt_to_phys(io_tlb_start) && paddr < io_tlb_end; + return paddr >= io_tlb_start && paddr < io_tlb_end; }
Quoted 6 lines
As far as the number of patches I decided to do this incrementally instead of trying to do it all at once. That way it is clearer to the reviewer what I am doing in each step and it can be more easily bisected in case I messed up somewhere. If you want fewer patches I can do that but I don't see the point in combining patches since they are all just going to result in the same total change anyway.
No that is OK. BTW, I did a testing of your V2 patches with Xen-SWIOTLB and they worked. But it was on non-debug env. The debug one does such evil things as make the initial domain memory start at the end physical memory and put the underlaying MFNs (machine frame numbers, the real physical frames) in reverse order. So for example pfn 100, ends up being mfn 0xf00d, and pfn 101 ends up being oxf00c.
Re: [PATCH v2 1/7] swiotlb: Make io_tlb_end a physical address instead of a virtual one
Quoted 17 lines · Konrad Rzeszutek Wilk
Quoted 16 lines · Alexander Duyck
end to be physical instead of virtual. I reviewed the code and realized that I wasn't saving anything by removing it since the overall code was larger as a result so I just converted it to a physical address. There are no users of
io_tlb_endthat are accessing it as a virtual value so all I did is just change it to a physical one and drop thevirt_to_physcalls that were made on it. If I am not mistaken by the second patch theis_swiotlb_buffercall is literally what you have described above. Here is the snippet from the 2nd patch:static int
is_swiotlb_buffer(phys_addr_t paddr)
{(quoted diff) +1
-1- return paddr >= virt_to_phys(io_tlb_start) && paddr < io_tlb_end; + return paddr >= io_tlb_start && paddr < io_tlb_end; }
Quoted 12 lines
Quoted 6 lines
As far as the number of patches I decided to do this incrementally instead of trying to do it all at once. That way it is clearer to the reviewer what I am doing in each step and it can be more easily bisected in case I messed up somewhere. If you want fewer patches I can do that but I don't see the point in combining patches since they are all just going to result in the same total change anyway.
No that is OK. BTW, I did a testing of your V2 patches with Xen-SWIOTLB and they worked. But it was on non-debug env. The debug one does such evil things as make the initial domain memory start at the end physical memory and put the underlaying MFNs (machine frame numbers, the real physical frames) in reverse order. So for example pfn 100, ends up being mfn
0xf00d, and pfn 101 ends up being oxf00c.
Glad to hear they are working under Xen. As I said I am pretty sure if it was working before it should still be working after these changes since the only difference is that instead of working with virtual addresses and having to call virt_to_phys to get the pseudo-physical addresses we are just working with the pseudo-physical addresses directly.
Thanks,
Alex
PATCH v2 2/7 swiotlb: Make io_tlb_start a physical address instead of a virtual one
In reply to Alexander DuyckThis change replaces all references to the virtual address for io_tlb_start with references to the physical address io_tlb_end. The main advantage of replacing the virtual address with a physical address is that we can avoid having to do multiple translations from the virtual address to the physical one needed for testing an existing DMA address.
---
lib/swiotlb.c | 58 +++++++++++++++++++++++++++++---------------------------- 1 files changed, 29 insertions(+), 29 deletions(-)
lib/swiotlb.c +29 -29
@@ -57,8 +57,7 @@ int swiotlb_force;* swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this * API. */ -static char *io_tlb_start; -phys_addr_t io_tlb_end; +phys_addr_t io_tlb_start, io_tlb_end;/* * The number of IO TLB blocks (in groups of 64) between io_tlb_start and
@@ -126,16 +125,15 @@ static dma_addr_t swiotlb_virt_to_bus(struct device *hwdev,void swiotlb_print_info (void ) {unsigned long bytes = io_tlb_nslabs <<IO_TLB_SHIFT ; -phys_addr_t pstart; -unsigned char *vend; +unsigned char *vstart, *vend; - pstart =virt_to_phys (io_tlb_start); + vstart =phys_to_virt (io_tlb_start); vend =phys_to_virt (io_tlb_end);printk (KERN_INFO "software IO TLB [mem %#010llx-%#010llx] (%luMB) mapped at [%p-%p]\n" , - (unsigned long long )pstart, + (unsigned long long )io_tlb_start, (unsigned long long )io_tlb_end, - bytes >>20 , io_tlb_start, vend -1 ); + bytes >>20 , vstart, vend -1 ); }void __init swiotlb_init_with_tbl (char *tlb,unsigned long nslabs,int verbose)
@@ -145,8 +143,8 @@ void __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) bytes = nslabs <<IO_TLB_SHIFT ; io_tlb_nslabs = nslabs; - io_tlb_start = tlb; - io_tlb_end =__pa (io_tlb_start) + bytes; + io_tlb_start =__pa (tlb); + io_tlb_end = io_tlb_start + bytes;/* * Allocate and initialize the free list array. This array is used
@@ -176,6 +174,7 @@ void __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)static void __init swiotlb_init_with_default_size (size_t default_size,int verbose) { +unsigned char *vstart;unsigned long bytes;if (!io_tlb_nslabs) {
@@ -188,11 +187,11 @@ swiotlb_init_with_default_size(size_t default_size, int verbose)/* * Get IO TLB memory from the low pages */ - io_tlb_start =alloc_bootmem_low_pages (PAGE_ALIGN (bytes)); -if (!io_tlb_start) + vstart =alloc_bootmem_low_pages (PAGE_ALIGN (bytes)); +if (!vstart)panic ("Cannot allocate SWIOTLB buffer" ); -swiotlb_init_with_tbl (io_tlb_start, io_tlb_nslabs, verbose); +swiotlb_init_with_tbl (vstart, io_tlb_nslabs, verbose); }void __init
@@ -210,6 +209,7 @@ int swiotlb_late_init_with_default_size(size_t default_size) {unsigned long bytes, req_nslabs = io_tlb_nslabs; +unsigned char *vstart =NULL ;unsigned int order;int rc =0 ;
@@ -226,14 +226,14 @@ swiotlb_late_init_with_default_size(size_t default_size) bytes = io_tlb_nslabs <<IO_TLB_SHIFT ;while ((SLABS_PER_PAGE << order) >IO_TLB_MIN_SLABS ) { - io_tlb_start = (void *)__get_free_pages (GFP_DMA | __GFP_NOWARN, - order); -if (io_tlb_start) + vstart = (void *)__get_free_pages (GFP_DMA | __GFP_NOWARN, + order); +if (vstart)break ; order--; } -if (!io_tlb_start) { +if (!vstart) { io_tlb_nslabs = req_nslabs;return -ENOMEM ; }
@@ -242,9 +242,9 @@ swiotlb_late_init_with_default_size(size_t default_size) "for software IO TLB\n", (PAGE_SIZE << order) >> 20); io_tlb_nslabs =SLABS_PER_PAGE << order; } - rc =swiotlb_late_init_with_tbl (io_tlb_start, io_tlb_nslabs); + rc =swiotlb_late_init_with_tbl (vstart, io_tlb_nslabs);if (rc) -free_pages ((unsigned long )io_tlb_start, order); +free_pages ((unsigned long )vstart, order);return rc; }
@@ -256,10 +256,10 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs) bytes = nslabs <<IO_TLB_SHIFT ; io_tlb_nslabs = nslabs; - io_tlb_start = tlb; - io_tlb_end =virt_to_phys (io_tlb_start) + bytes; + io_tlb_start =virt_to_phys (tlb); + io_tlb_end = io_tlb_start + bytes; -memset (io_tlb_start,0 , bytes); +memset (tlb,0 , bytes);/* * Allocate and initialize the free list array. This array is used
@@ -308,7 +308,7 @@ cleanup3: io_tlb_list =NULL ; cleanup2: io_tlb_end =0 ; - io_tlb_start =NULL ; + io_tlb_start =0 ; io_tlb_nslabs =0 ;return -ENOMEM ; }
@@ -325,7 +325,7 @@ void __init swiotlb_free(void) get_order(io_tlb_nslabs * sizeof(phys_addr_t)));free_pages ((unsigned long )io_tlb_list,get_order (io_tlb_nslabs *sizeof (int ))); -free_pages ((unsigned long )io_tlb_start, +free_pages ((unsigned long )phys_to_virt (io_tlb_start),get_order (io_tlb_nslabs <<IO_TLB_SHIFT )); }else { free_bootmem_late(__pa(io_tlb_overflow_buffer),
@@ -334,7 +334,7 @@ void __init swiotlb_free(void) PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)));free_bootmem_late (__pa (io_tlb_list),PAGE_ALIGN (io_tlb_nslabs *sizeof (int ))); -free_bootmem_late (__pa (io_tlb_start), +free_bootmem_late (io_tlb_start,PAGE_ALIGN (io_tlb_nslabs <<IO_TLB_SHIFT )); } io_tlb_nslabs =0 ;
@@ -342,7 +342,7 @@ void __init swiotlb_free(void)static int is_swiotlb_buffer (phys_addr_t paddr) { -return paddr >=virt_to_phys (io_tlb_start) && paddr < io_tlb_end; +return paddr >= io_tlb_start && paddr < io_tlb_end; }/*
@@ -455,7 +455,7 @@ void *swiotlb_tbl_map_single(struct device *hwdev, dma_addr_t tbl_dma_addr, io_tlb_list[i] =0 ;for (i = index -1 ; (OFFSET (i,IO_TLB_SEGSIZE ) !=IO_TLB_SEGSIZE -1 ) && io_tlb_list[i]; i--) io_tlb_list[i] = ++count; - dma_addr = io_tlb_start + (index <<IO_TLB_SHIFT ); + dma_addr = (char *)phys_to_virt (io_tlb_start) + (index <<IO_TLB_SHIFT );/* * Update the indices to avoid searching in the next
@@ -499,7 +499,7 @@ static void * map_single(struct device *hwdev, phys_addr_t phys, size_t size, enum dma_data_direction dir) { - dma_addr_t start_dma_addr = swiotlb_virt_to_bus(hwdev, io_tlb_start); + dma_addr_t start_dma_addr = phys_to_dma(hwdev, io_tlb_start);return swiotlb_tbl_map_single (hwdev, start_dma_addr, phys, size, dir); }
@@ -513,7 +513,7 @@ swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size, {unsigned long flags;int i, count, nslots =ALIGN (size,1 <<IO_TLB_SHIFT ) >>IO_TLB_SHIFT ; -int index = (dma_addr - io_tlb_start) >>IO_TLB_SHIFT ; +int index = (dma_addr - (char *)phys_to_virt (io_tlb_start)) >>IO_TLB_SHIFT ;phys_addr_t phys = io_tlb_orig_addr[index];/*
@@ -554,7 +554,7 @@ swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr, size_t size,enum dma_data_direction dir, enum dma_sync_target target) { - int index = (dma_addr - io_tlb_start) >> IO_TLB_SHIFT; + int index = (dma_addr - (char *)phys_to_virt(io_tlb_start)) >> IO_TLB_SHIFT;phys_addr_t phys = io_tlb_orig_addr[index]; phys += ((unsigned long )dma_addr & ((1 <<IO_TLB_SHIFT ) -1 ));
PATCH v2 3/7 swiotlb: Make io_tlb_overflow_buffer a physical address
In reply to Alexander DuyckThis change makes it so that we can avoid virt_to_phys overhead when using the io_tlb_overflow_buffer. My original plan was to completely remove the value and replace it with a constant but I had seen that there were recent patches that stated this couldn't be done until all device drivers that depended on that functionality be updated.
---
lib/swiotlb.c | 61 ++++++++++++++++++++++++++++++++------------------------- 1 files changed, 34 insertions(+), 27 deletions(-)
lib/swiotlb.c +34 -27
@@ -70,7 +70,7 @@ static unsigned long io_tlb_nslabs;*/ static unsigned long io_tlb_overflow =32 *1024 ; -static void *io_tlb_overflow_buffer; +phys_addr_t io_tlb_overflow_buffer;/* * This is a free list describing the number of free entries available from
@@ -138,6 +138,7 @@ void swiotlb_print_info(void)void __init swiotlb_init_with_tbl (char *tlb,unsigned long nslabs,int verbose) { +void *v_overflow_buffer;unsigned long i, bytes; bytes = nslabs <<IO_TLB_SHIFT ;
@@ -147,6 +148,15 @@ void __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) io_tlb_end = io_tlb_start + bytes;/* +* Get the overflow emergency buffer +*/ + v_overflow_buffer =alloc_bootmem_low_pages (PAGE_ALIGN (io_tlb_overflow)); +if (!v_overflow_buffer) +panic ("Cannot allocate SWIOTLB overflow buffer!\n" ); + + io_tlb_overflow_buffer =__pa (v_overflow_buffer); + +/* * Allocate and initialize the free list array. This array is used * to find contiguous free memory regions of size up to IO_TLB_SEGSIZE * between io_tlb_start and io_tlb_end.
@@ -157,12 +167,6 @@ void __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) io_tlb_index =0 ; io_tlb_orig_addr =alloc_bootmem_pages (PAGE_ALIGN (io_tlb_nslabs *sizeof (phys_addr_t))); -/* -* Get the overflow emergency buffer -*/ - io_tlb_overflow_buffer =alloc_bootmem_low_pages (PAGE_ALIGN (io_tlb_overflow)); -if (!io_tlb_overflow_buffer) -panic ("Cannot allocate SWIOTLB overflow buffer!\n" );if (verbose)swiotlb_print_info (); }
@@ -252,6 +256,7 @@ int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs) {unsigned long i, bytes; +unsigned char *v_overflow_buffer; bytes = nslabs <<IO_TLB_SHIFT ;
@@ -262,6 +267,16 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)memset (tlb,0 , bytes);/* +* Get the overflow emergency buffer +*/ + v_overflow_buffer = (void *)__get_free_pages (GFP_DMA , +get_order (io_tlb_overflow)); +if (!v_overflow_buffer) + goto cleanup2; + + io_tlb_overflow_buffer =virt_to_phys (v_overflow_buffer); + +/* * Allocate and initialize the free list array. This array is used * to find contiguous free memory regions of size up to IO_TLB_SEGSIZE * between io_tlb_start and io_tlb_end.
@@ -269,7 +284,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs) io_tlb_list = (unsigned int *)__get_free_pages (GFP_KERNEL ,get_order (io_tlb_nslabs *sizeof (int )));if (!io_tlb_list) - goto cleanup2; + goto cleanup3;for (i =0 ; i < io_tlb_nslabs; i++) io_tlb_list[i] =IO_TLB_SEGSIZE -OFFSET (i,IO_TLB_SEGSIZE );
@@ -280,18 +295,10 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)get_order (io_tlb_nslabs * sizeof(phys_addr_t)));if (!io_tlb_orig_addr) - goto cleanup3; + goto cleanup4;memset (io_tlb_orig_addr,0 , io_tlb_nslabs *sizeof (phys_addr_t)); -/* -* Get the overflow emergency buffer -*/ - io_tlb_overflow_buffer = (void *)__get_free_pages (GFP_DMA , -get_order (io_tlb_overflow)); -if (!io_tlb_overflow_buffer) - goto cleanup4; -swiotlb_print_info (); late_alloc =1 ;
@@ -299,13 +306,13 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)return 0 ; cleanup4: -free_pages ((unsigned long )io_tlb_orig_addr, -get_order (io_tlb_nslabs *sizeof (phys_addr_t))); - io_tlb_orig_addr =NULL ; -cleanup3:free_pages ((unsigned long )io_tlb_list,get_order (io_tlb_nslabs *sizeof (int ))); io_tlb_list =NULL ; +cleanup3: +free_pages ((unsigned long )v_overflow_buffer, +get_order (io_tlb_overflow)); + io_tlb_overflow_buffer =0 ; cleanup2: io_tlb_end =0 ; io_tlb_start =0 ;
@@ -315,11 +322,11 @@ cleanup2:void __init swiotlb_free (void ) { -if (!io_tlb_overflow_buffer) +if (!io_tlb_orig_addr)return ;if (late_alloc) { -free_pages ((unsigned long )io_tlb_overflow_buffer, +free_pages ((unsigned long )phys_to_virt (io_tlb_overflow_buffer),get_order (io_tlb_overflow));free_pages ((unsigned long )io_tlb_orig_addr,get_order (io_tlb_nslabs *sizeof (phys_addr_t)));
@@ -328,7 +335,7 @@ void __init swiotlb_free(void)free_pages ((unsigned long )phys_to_virt (io_tlb_start),get_order (io_tlb_nslabs <<IO_TLB_SHIFT )); }else { -free_bootmem_late (__pa (io_tlb_overflow_buffer), +free_bootmem_late (io_tlb_overflow_buffer,PAGE_ALIGN (io_tlb_overflow));free_bootmem_late (__pa (io_tlb_orig_addr),PAGE_ALIGN (io_tlb_nslabs *sizeof (phys_addr_t)));
@@ -698,7 +705,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page, map =map_single (dev, phys, size, dir);if (!map) {swiotlb_full (dev, size, dir,1 ); - map = io_tlb_overflow_buffer; +return phys_to_dma (dev, io_tlb_overflow_buffer); } dev_addr =swiotlb_virt_to_bus (dev, map);
@@ -708,7 +715,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,*/ if (!dma_capable (dev, dev_addr, size)) {swiotlb_tbl_unmap_single (dev, map, size, dir); - dev_addr =swiotlb_virt_to_bus (dev, io_tlb_overflow_buffer); +return phys_to_dma (dev, io_tlb_overflow_buffer); }return dev_addr;
@@ -927,7 +934,7 @@ EXPORT_SYMBOL(swiotlb_sync_sg_for_device);int swiotlb_dma_mapping_error (struct device *hwdev,dma_addr_t dma_addr) { -return (dma_addr ==swiotlb_virt_to_bus (hwdev, io_tlb_overflow_buffer)); +return (dma_addr ==phys_to_dma (hwdev, io_tlb_overflow_buffer)); }EXPORT_SYMBOL (swiotlb_dma_mapping_error);
PATCH v2 4/7 swiotlb: Return physical addresses when calling swiotlb_tbl_map_single
In reply to Alexander DuyckThis change makes it so that swiotlb_tbl_map_single will return a physical address instead of a virtual address when called. The advantage to this once again is that we are avoiding a number of virt_to_phys and phys_to_virt translations by working with everything as a physical address.
One change I had to make in order to support using physical addresses is that I could no longer trust 0 to be a invalid physical address on all platforms. So instead I made it so that ~0 is returned on error. This should never be a valid return value as it implies that only one byte would be available for use.
In order to clarify things since we now have 2 physical addresses in use inside of swiotlb_tbl_map_single I am renaming phys to orig_addr, and dma_addr to tlb_addr. This way is should be clear that orig_addr is contained within io_orig_addr and tlb_addr is an address within the io_tlb_addr buffer.
---
drivers/xen/swiotlb-xen.c | 22 ++++++------- include/linux/swiotlb.h | 11 +++++- lib/swiotlb.c | 78 +++++++++++++++++++++++---------------------- 3 files changed, 59 insertions(+), 52 deletions(-)
drivers/xen/swiotlb-xen.c +11 -11
@@ -338,9 +338,8 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,enum dma_data_direction dir, struct dma_attrs *attrs) { - phys_addr_t phys = page_to_phys(page) + offset; + phys_addr_t map, phys = page_to_phys(page) + offset;dma_addr_t dev_addr =xen_phys_to_bus (phys); -void *map;BUG_ON (dir ==DMA_NONE );/*
@@ -356,16 +355,16 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,* Oh well, have to allocate and map a bounce buffer. */ map =swiotlb_tbl_map_single (dev, start_dma_addr, phys, size, dir); -if (!map) +if (map ==SWIOTLB_MAP_ERROR )return DMA_ERROR_CODE ; - dev_addr =xen_virt_to_bus (map); + dev_addr =xen_phys_to_bus (map);/* * Ensure that the address returned is DMA'ble */ if (!dma_capable (dev, dev_addr, size)) { -swiotlb_tbl_unmap_single (dev, map, size, dir); +swiotlb_tbl_unmap_single (dev,phys_to_virt (map), size, dir); dev_addr =0 ; }return dev_addr;
@@ -494,11 +493,12 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,if (swiotlb_force || !dma_capable (hwdev, dev_addr, sg->length) ||range_straddles_page_boundary (paddr, sg->length)) { -void *map =swiotlb_tbl_map_single (hwdev, - start_dma_addr, -sg_phys (sg), - sg->length, dir); -if (!map) { +phys_addr_t map =swiotlb_tbl_map_single (hwdev, + start_dma_addr, +sg_phys (sg), + sg->length, + dir); +if (map ==SWIOTLB_MAP_ERROR ) {/* Don't panic here, we expect map_sg users to do proper error handling. */ xen_swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
@@ -506,7 +506,7 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, sgl[0 ].dma_length =0 ;return DMA_ERROR_CODE ; } - sg->dma_address =xen_virt_to_bus (map); + sg->dma_address =xen_phys_to_bus (map); }else sg->dma_address = dev_addr; sg->dma_length = sg->length;
include/linux/swiotlb.h +8 -3
@@ -34,9 +34,14 @@ enum dma_sync_target {SYNC_FOR_CPU =0 , SYNC_FOR_DEVICE = 1, }; -extern void *swiotlb_tbl_map_single (struct device *hwdev,dma_addr_t tbl_dma_addr, -phys_addr_t phys,size_t size, -enum dma_data_direction dir); + +/* define the last possible byte of physical address space as a mapping error */ +#define SWIOTLB_MAP_ERROR (~(phys_addr_t)0x0) + +extern phys_addr_t swiotlb_tbl_map_single (struct device *hwdev, +dma_addr_t tbl_dma_addr, +phys_addr_t phys,size_t size, +enum dma_data_direction dir);extern void swiotlb_tbl_unmap_single (struct device *hwdev,char *dma_addr,size_t size,enum dma_data_direction dir);
lib/swiotlb.c +40 -38
165 lines not on this page · raw message
PATCH v2 5/7 swiotlb: Use physical addresses for swiotlb_tbl_unmap_single
In reply to Alexander DuyckThis change makes it so that the unmap functionality also uses physical addresses. This helps to further reduce the use of virt_to_phys and phys_to_virt functions.
In order to clarify things since we now have 2 physical addresses in use inside of swiotlb_tbl_unmap_single I am renaming phys to orig_addr, and dma_addr to tlb_addr. This way is should be clear that orig_addr is contained within io_orig_addr and tlb_addr is an address within the io_tlb_addr buffer.
---
drivers/xen/swiotlb-xen.c | 4 ++-- include/linux/swiotlb.h | 3 ++- lib/swiotlb.c | 37 +++++++++++++++++++------------------ 3 files changed, 23 insertions(+), 21 deletions(-)
drivers/xen/swiotlb-xen.c +2 -2
@@ -364,7 +364,7 @@ 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,phys_to_virt (map), size, dir); +swiotlb_tbl_unmap_single (dev, map, size, dir); dev_addr =0 ; }return dev_addr;
@@ -388,7 +388,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,phys_to_virt (paddr), size, dir); +swiotlb_tbl_unmap_single (hwdev, paddr, size, dir);return ; }
include/linux/swiotlb.h +2 -1
@@ -43,7 +43,8 @@ extern phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys, size_t size, enum dma_data_direction dir); -extern void swiotlb_tbl_unmap_single (struct device *hwdev,char *dma_addr, +extern void swiotlb_tbl_unmap_single (struct device *hwdev, +phys_addr_t tlb_addr,size_t size,enum dma_data_direction dir); extern void swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr,
lib/swiotlb.c +19 -18
@@ -515,20 +515,20 @@ phys_addr_t map_single(struct device *hwdev, phys_addr_t phys, size_t size,/* * dma_addr is the kernel virtual address of the bounce buffer to unmap. */ -void -swiotlb_tbl_unmap_single (struct device *hwdev,char *dma_addr,size_t size, -enum dma_data_direction dir) +void swiotlb_tbl_unmap_single (struct device *hwdev,phys_addr_t tlb_addr, +size_t size,enum dma_data_direction dir) {unsigned long flags;int i, count, nslots =ALIGN (size,1 <<IO_TLB_SHIFT ) >>IO_TLB_SHIFT ; -int index = (dma_addr - (char *)phys_to_virt (io_tlb_start)) >>IO_TLB_SHIFT ; -phys_addr_t phys = io_tlb_orig_addr[index]; +int index = (tlb_addr - io_tlb_start) >>IO_TLB_SHIFT ; +phys_addr_t orig_addr = io_tlb_orig_addr[index];/* * First, sync the memory before unmapping the entry */ if (phys && ((dir ==DMA_FROM_DEVICE ) || (dir ==DMA_BIDIRECTIONAL ))) -swiotlb_bounce (phys, dma_addr, size,DMA_FROM_DEVICE ); +swiotlb_bounce (orig_addr,phys_to_virt (tlb_addr), + size,DMA_FROM_DEVICE );/* * Return the buffer to the free list by setting the corresponding
@@ -621,17 +621,18 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size, ret =phys_to_virt (paddr); dev_addr =phys_to_dma (hwdev, paddr); - } -/* Confirm address can be DMA'd by device */ -if (dev_addr + size -1 > dma_mask) { -printk ("hwdev DMA mask = 0x%016Lx, dev_addr = 0x%016Lx\n" , - (unsigned long long )dma_mask, - (unsigned long long )dev_addr); +/* Confirm address can be DMA'd by device */ +if (dev_addr + size -1 > dma_mask) { +printk ("hwdev DMA mask = 0x%016Lx, dev_addr = 0x%016Lx\n" , + (unsigned long long )dma_mask, + (unsigned long long )dev_addr); -/* DMA_TO_DEVICE to avoid memcpy in unmap_single */ -swiotlb_tbl_unmap_single (hwdev, ret, size,DMA_TO_DEVICE ); -return NULL ; +/* DMA_TO_DEVICE to avoid memcpy in unmap_single */ +swiotlb_tbl_unmap_single (hwdev, paddr, + size,DMA_TO_DEVICE ); +return NULL ; + } } *dma_handle = dev_addr;
@@ -652,7 +653,7 @@ swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,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 ,vaddr ,size ,DMA_TO_DEVICE ); +swiotlb_tbl_unmap_single (hwdev ,paddr ,size ,DMA_TO_DEVICE ); }EXPORT_SYMBOL (swiotlb_free_coherent);
@@ -716,7 +717,7 @@ dma_addr_t 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,phys_to_virt (map), size, dir); +swiotlb_tbl_unmap_single (dev, map, size, dir);return phys_to_dma (dev, io_tlb_overflow_buffer); }
@@ -740,7 +741,7 @@ static void unmap_single(struct device *hwdev, dma_addr_t dev_addr,BUG_ON (dir ==DMA_NONE );if (is_swiotlb_buffer (paddr)) { -swiotlb_tbl_unmap_single (hwdev,phys_to_virt (paddr), size, dir); +swiotlb_tbl_unmap_single (hwdev, paddr, size, dir);return ; }
PATCH v2 6/7 swiotlb: Use physical addresses instead of virtual in swiotlb_tbl_sync_single
In reply to Alexander DuyckThis change makes it so that the sync functionality also uses physical addresses. This helps to further reduce the use of virt_to_phys and phys_to_virt functions.
In order to clarify things since we now have 2 physical addresses in use inside of swiotlb_tbl_sync_single I am renaming phys to orig_addr, and dma_addr to tlb_addr. This way is should be clear that orig_addr is contained within io_orig_addr and tlb_addr is an address within the io_tlb_addr buffer.
---
drivers/xen/swiotlb-xen.c | 3 +-- include/linux/swiotlb.h | 3 ++- lib/swiotlb.c | 22 +++++++++++----------- 3 files changed, 14 insertions(+), 14 deletions(-)
drivers/xen/swiotlb-xen.c +1 -2
@@ -433,8 +433,7 @@ xen_swiotlb_sync_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_sync_single (hwdev,phys_to_virt (paddr), size, dir, - target); +swiotlb_tbl_sync_single (hwdev, paddr, size, dir, target);return ; }
include/linux/swiotlb.h +2 -1
@@ -47,7 +47,8 @@ extern void swiotlb_tbl_unmap_single(struct device *hwdev,phys_addr_t tlb_addr, size_t size, enum dma_data_direction dir); -extern void swiotlb_tbl_sync_single (struct device *hwdev,char *dma_addr, +extern void swiotlb_tbl_sync_single (struct device *hwdev, +phys_addr_t tlb_addr,size_t size,enum dma_data_direction dir,enum dma_sync_target target);
lib/swiotlb.c +11 -11
@@ -557,26 +557,27 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr, }EXPORT_SYMBOL_GPL (swiotlb_tbl_unmap_single); -void -swiotlb_tbl_sync_single (struct device *hwdev,char *dma_addr,size_t size, -enum dma_data_direction dir, -enum dma_sync_target target) +void swiotlb_tbl_sync_single (struct device *hwdev,phys_addr_t tlb_addr, +size_t size,enum dma_data_direction dir, +enum dma_sync_target target) { -int index = (dma_addr - (char *)phys_to_virt (io_tlb_start)) >>IO_TLB_SHIFT ; -phys_addr_t phys = io_tlb_orig_addr[index]; +int index = (tlb_addr - io_tlb_start) >>IO_TLB_SHIFT ; +phys_addr_t orig_addr = io_tlb_orig_addr[index]; - phys += ((unsigned long )dma_addr & ((1 <<IO_TLB_SHIFT ) -1 )); + orig_addr += (unsigned long )tlb_addr & ((1 <<IO_TLB_SHIFT ) -1 );switch (target) {case SYNC_FOR_CPU :if (likely (dir ==DMA_FROM_DEVICE || dir ==DMA_BIDIRECTIONAL )) -swiotlb_bounce (phys, dma_addr, size,DMA_FROM_DEVICE ); +swiotlb_bounce (orig_addr,phys_to_virt (tlb_addr), + size,DMA_FROM_DEVICE );else BUG_ON (dir !=DMA_TO_DEVICE );break ;case SYNC_FOR_DEVICE :if (likely (dir ==DMA_TO_DEVICE || dir ==DMA_BIDIRECTIONAL )) -swiotlb_bounce (phys, dma_addr, size,DMA_TO_DEVICE ); +swiotlb_bounce (orig_addr,phys_to_virt (tlb_addr), + size,DMA_TO_DEVICE );else BUG_ON (dir !=DMA_FROM_DEVICE );break ;
@@ -785,8 +786,7 @@ swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,BUG_ON (dir ==DMA_NONE );if (is_swiotlb_buffer (paddr)) { -swiotlb_tbl_sync_single (hwdev,phys_to_virt (paddr), size, dir, - target); +swiotlb_tbl_sync_single (hwdev, paddr, size, dir, target);return ; }
PATCH v2 7/7 swiotlb: Do not export swiotlb_bounce since there are no external consumers
In reply to Alexander DuyckCurrently swiotlb is the only consumer for swiotlb_bounce. Since that is the case it doesn't make much sense to be exporting it so make it a static function only.
In addition we can save a few more lines of code by making it so that it accepts the DMA address as a physical address instead of a virtual one. This is the last piece in essentially pushing all of the DMA address values to use physical addresses in swiotlb.
In order to clarify things since we now have 2 physical addresses in use inside of swiotlb_bounce I am renaming phys to orig_addr, and dma_addr to tlb_addr. This way is should be clear that orig_addr is contained within io_orig_addr and tlb_addr is an address within the io_tlb_addr buffer.
---
include/linux/swiotlb.h | 3 --- lib/swiotlb.c | 35 ++++++++++++++++------------------- 2 files changed, 16 insertions(+), 22 deletions(-)
include/linux/swiotlb.h +0 -3
@@ -53,9 +53,6 @@ extern void swiotlb_tbl_sync_single(struct device *hwdev, enum dma_sync_target target);/* Accessory functions. */ -extern void swiotlb_bounce (phys_addr_t phys,char *dma_addr,size_t size, -enum dma_data_direction dir); -extern void *swiotlb_alloc_coherent (struct device *hwdev,size_t size,dma_addr_t *dma_handle,gfp_t flags);
lib/swiotlb.c +16 -19
@@ -355,14 +355,15 @@ static int is_swiotlb_buffer(phys_addr_t paddr)/* * Bounce: copy the swiotlb buffer back to the original dma location */ -void swiotlb_bounce (phys_addr_t phys,char *dma_addr,size_t size, -enum dma_data_direction dir) +static void swiotlb_bounce (phys_addr_t orig_addr,phys_addr_t tlb_addr, +size_t size,enum dma_data_direction dir) { -unsigned long pfn =PFN_DOWN (phys); +unsigned long pfn =PFN_DOWN (orig_addr); +unsigned char *vaddr =phys_to_virt (tlb_addr);if (PageHighMem (pfn_to_page (pfn))) {/* The buffer does not have a mapping. Map it in and copy */ -unsigned int offset = phys & ~PAGE_MASK ; +unsigned int offset = orig_addr & ~PAGE_MASK ;char *buffer;unsigned int sz =0 ;unsigned long flags;
@@ -373,25 +374,23 @@ void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,local_irq_save (flags); buffer =kmap_atomic (pfn_to_page (pfn));if (dir ==DMA_TO_DEVICE ) -memcpy (dma_addr, buffer + offset, sz); +memcpy (vaddr, buffer + offset, sz);else -memcpy (buffer + offset, dma_addr, sz); +memcpy (buffer + offset, vaddr, sz);kunmap_atomic (buffer);local_irq_restore (flags); size -= sz; pfn++; - dma_addr += sz; + vaddr += sz; offset =0 ; } + }else if (dir ==DMA_TO_DEVICE ) { +memcpy (vaddr,phys_to_virt (orig_addr), size); }else { -if (dir ==DMA_TO_DEVICE ) -memcpy (dma_addr,phys_to_virt (phys), size); -else -memcpy (phys_to_virt (phys), dma_addr, size); +memcpy (phys_to_virt (orig_addr), vaddr, size); } } -EXPORT_SYMBOL_GPL (swiotlb_bounce); phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, dma_addr_t tbl_dma_addr,
@@ -493,8 +492,7 @@ found:for (i =0 ; i < nslots; i++) io_tlb_orig_addr[index+i] = orig_addr + (i <<IO_TLB_SHIFT );if (dir ==DMA_TO_DEVICE || dir ==DMA_BIDIRECTIONAL ) -swiotlb_bounce (orig_addr,phys_to_virt (tlb_addr), size, -DMA_TO_DEVICE ); +swiotlb_bounce (orig_addr, tlb_addr, size,DMA_TO_DEVICE );return tlb_addr; }
@@ -526,9 +524,8 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,/* * First, sync the memory before unmapping the entry */ -if (phys && ((dir ==DMA_FROM_DEVICE ) || (dir ==DMA_BIDIRECTIONAL ))) -swiotlb_bounce (orig_addr,phys_to_virt (tlb_addr), - size,DMA_FROM_DEVICE ); +if (orig_addr && ((dir ==DMA_FROM_DEVICE ) || (dir ==DMA_BIDIRECTIONAL ))) +swiotlb_bounce (orig_addr, tlb_addr, size,DMA_FROM_DEVICE );/* * Return the buffer to the free list by setting the corresponding
@@ -569,14 +566,14 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,switch (target) {case SYNC_FOR_CPU :if (likely (dir ==DMA_FROM_DEVICE || dir ==DMA_BIDIRECTIONAL )) -swiotlb_bounce (orig_addr,phys_to_virt (tlb_addr), +swiotlb_bounce (orig_addr, tlb_addr, size,DMA_FROM_DEVICE );else BUG_ON (dir !=DMA_TO_DEVICE );break ;case SYNC_FOR_DEVICE :if (likely (dir ==DMA_TO_DEVICE || dir ==DMA_BIDIRECTIONAL )) -swiotlb_bounce (orig_addr,phys_to_virt (tlb_addr), +swiotlb_bounce (orig_addr, tlb_addr, size,DMA_TO_DEVICE );else BUG_ON (dir !=DMA_FROM_DEVICE );
Quoted 37 lines · Alexander Duyck
While working on 10Gb/s routing performance I found a significant amount of time was being spent in the swiotlb DMA handler. Further digging found that a significant amount of this was due to virtual to physical address translation and calling the function that did it. It accounted for nearly 60% of the total swiotlb overhead.
This patch set works to resolve that by replacing the
io_tlb_startandio_tlb_endvirtual addresses with a physical addresses. In addition it changes theio_tlb_overflow_bufferfrom a virtual to a physical address. I followed through with the cleanup to the point that the only functions that really require the virtual address for the DMA buffer are the init, free, and bounce functions.In the case of devices that are using the bounce buffers these patches should result in only a slight performance gain if any. This is due to the locking overhead required to map and unmap the buffers.
In the case of devices that are not making use of bounce buffers these patches can significantly reduce their overhead. In the case of an ixgbe routing test for example, these changes result in 7 fewer calls to
__phys_addrand allowis_swiotlb_bufferto become inlined due to a reduction in the number of instructions. When running a routing throughput test using small packets I saw roughly a 6% increase in packets rates after applying these patches. This appears to match up with the CPU overhead reduction I was tracking via perf.Before:
Results 10.0MppsAfter:
Results 10.6MppsFinally, I updated the parameter names for several of the core function calls as there was some ambiguity in naming. Specifically virtual address pointers were named
dma_addr. When I changed these pointers to physical I instead used the nametlb_addras this value represented a physical address in theio_tlb_startregion and is less likely to be confused with a bus address.
Quoted 8 lines
v2:
I reviewed the changes and realized that the first patch that was droppingio_tlb_endand calculating the value didn't actually gain me much once I had gone through and translated the rest of the addresses to physical addresses. As such I have updated the patch so that it instead is convertingio_tlb_endfrom a virtual address to a physical address. This actually helps to reduce the overhead foris_swiotlb_bufferandswiotlb_dma_supportedby several instructions.
The first three patches were replacing "static char *" with "phys_addr_t" when I should have been using "static phys_addr_t". As such I will submit an updated v3 version on Monday.
Thanks,
Alex