Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933660AbbBIRUZ (ORCPT ); Mon, 9 Feb 2015 12:20:25 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:17945 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933558AbbBIRUY (ORCPT ); Mon, 9 Feb 2015 12:20:24 -0500 Date: Mon, 9 Feb 2015 10:35:54 -0500 From: Konrad Rzeszutek Wilk To: "Wang, Xiaoming" Cc: "ralf@linux-mips.org" , "boris.ostrovsky@oracle.com" , "david.vrabel@citrix.com" , "linux-mips@linux-mips.org" , "linux-kernel@vger.kernel.org" , "xen-devel@lists.xenproject.org" , "akpm@linux-foundation.org" , "linux@horizon.com" , "lauraa@codeaurora.org" , "heiko.carstens@de.ibm.com" , "d.kasatkin@samsung.com" , "takahiro.akashi@linaro.org" , "chris@chris-wilson.co.uk" , "pebolle@tiscali.nl" , "Liu, Chuansheng" , "Zhang, Dongxing" Subject: Re: [PATCH] modify the IO_TLB_SEGSIZE to io_tlb_segsize configurable as flexible requirement about SW-IOMMU. Message-ID: <20150209153554.GC8758@l.oracle.com> References: <1423177274-22118-1-git-send-email-xiaoming.wang@intel.com> <20150205193241.GC11646@x230.dumpdata.com> <20150206181149.GB16455@l.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Source-IP: acsinet21.oracle.com [141.146.126.237] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 15532 Lines: 353 On Mon, Feb 09, 2015 at 02:13:30AM +0000, Wang, Xiaoming wrote: > Dear Wilk: > > > -----Original Message----- > > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com] > > Sent: Saturday, February 7, 2015 2:12 AM > > To: Wang, Xiaoming > > Cc: ralf@linux-mips.org; boris.ostrovsky@oracle.com; > > david.vrabel@citrix.com; linux-mips@linux-mips.org; linux- > > kernel@vger.kernel.org; xen-devel@lists.xenproject.org; akpm@linux- > > foundation.org; linux@horizon.com; lauraa@codeaurora.org; > > heiko.carstens@de.ibm.com; d.kasatkin@samsung.com; > > takahiro.akashi@linaro.org; chris@chris-wilson.co.uk; pebolle@tiscali.nl; Liu, > > Chuansheng; Zhang, Dongxing > > Subject: Re: [PATCH] modify the IO_TLB_SEGSIZE to io_tlb_segsize > > configurable as flexible requirement about SW-IOMMU. > > > > On Fri, Feb 06, 2015 at 12:10:15AM +0000, Wang, Xiaoming wrote: > > > > > > > > > > -----Original Message----- > > > > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com] > > > > Sent: Friday, February 6, 2015 3:33 AM > > > > To: Wang, Xiaoming > > > > Cc: ralf@linux-mips.org; boris.ostrovsky@oracle.com; > > > > david.vrabel@citrix.com; linux-mips@linux-mips.org; linux- > > > > kernel@vger.kernel.org; xen-devel@lists.xenproject.org; akpm@linux- > > > > foundation.org; linux@horizon.com; lauraa@codeaurora.org; > > > > heiko.carstens@de.ibm.com; d.kasatkin@samsung.com; > > > > takahiro.akashi@linaro.org; chris@chris-wilson.co.uk; > > > > pebolle@tiscali.nl; Liu, Chuansheng; Zhang, Dongxing > > > > Subject: Re: [PATCH] modify the IO_TLB_SEGSIZE to io_tlb_segsize > > > > configurable as flexible requirement about SW-IOMMU. > > > > > > > > On Fri, Feb 06, 2015 at 07:01:14AM +0800, xiaomin1 wrote: > > > > > The maximum of SW-IOMMU is limited to 2^11*128 = 256K. > > > > > While in different platform and different requirements this seems > > improper. > > > > > So modify the IO_TLB_SEGSIZE to io_tlb_segsize as configurable is > > > > > make > > > > sense. > > > > > > > > More details please. What is the issue you are hitting? > > > > > > > Example: > > > If 1M bytes are requied. There has an error like. > > > > Ok, but even with 1MB size - you only have 64 'slots' (if you allocate an 64MB > > buffer). And the other 'slots' can be fragmented so you might still not have > > enough 1MB chunks available. > > > > Do you have some thoughts on how that would be addressed? > > > Yes, > If IO_TLB_SEGSIZE is 128 the slabs is 32K/128 = 256 > While IO_TLB_SEGSIZE is 512 the slabs is 32K/512 =64 (for 1M). > So it is dilemma between slabs and segsize. Right. > I have a thought how about modifying the IO_TLB_DEFAULT_SIZE to > io_tlb_default_size configurable too? It would seem that 'io_tlb_default_size' should be influenced by the 'io_tlb_segsize' - as in have some calculation that would come up with the best value (if there is one?) > Because of the multivariate requirement. > > > > [ 31.474769] dwc3_otg 0000:00:16.0: > > dwc3_intel_byt_notify_charger_type(): dwc3_intel_byt_notify_charger_type: > > invalid SDP current! > > > [ 31.554077] android_work: sent uevent USB_STATE=CONNECTED > > > [ 31.564244] android_usb gadget: high-speed config #1: android > > > [ 31.571468] android_work: sent uevent USB_STATE=CONFIGURED > > > [ 31.942738] DMA: Out of SW-IOMMU space for 1048576 bytes at device > > gadget > > > [ 31.950345] Kernel panic - not syncing: DMA: Random memory could be > > DMA written > > > [ 31.950345] > > > [ 31.960170] CPU: 1 PID: 172 Comm: droidboot Tainted: G W > > 3.10.20-x86_64_byt-g1077f87 #2 > > > [ 31.970086] Hardware name: Intel Corp. VALLEYVIEW C0 > > PLATFORM/BYT-T FFD8, BIOS BLADE_21.X64.0004.R14.1412311144 > > FFD8_X64_R_2014_12_31_1151 12/31/2014 > > > [ 31.985053] 0000000000100000 ffff880136c2fc98 ffffffff82967d45 > > ffff880136c2fd10 > > > [ 31.993327] ffffffff82961761 0000000000000008 ffff880136c2fd20 > > ffff880136c2fcc0 > > > [ 32.001590] ffffffff829618fb 0000000000000002 ffffffff820aeff9 > > 0000000000008d8c > > > [ 32.009871] Call Trace: > > > [ 32.012610] [] dump_stack+0x19/0x1b > > > [ 32.018353] [] panic+0xc8/0x1d6 > > > [ 32.023707] [] ? printk+0x55/0x57 > > > [ 32.029258] [] ? console_unlock+0x1f9/0x460 > > > [ 32.035772] [] swiotlb_map_page+0x12e/0x140 > > > [ 32.042283] [] > > usb_gadget_map_request+0x16d/0x220 > > > [ 32.049387] [] dwc3_gadget_ep_queue+0x229/0x460 > > > [ 32.056297] [] ffs_epfile_io.isra.96+0x3e4/0x520 > > > [ 32.063296] [] ? get_parent_ip+0xd/0x50 > > > [ 32.069427] [] ? sub_preempt_count+0x71/0x100 > > > [ 32.076142] [] ffs_epfile_read+0x28/0x30 > > > [ 32.082370] [] vfs_read+0x9c/0x170 > > > [ 32.088014] [] SyS_read+0x4d/0xa0 > > > [ 32.093562] [] ia32_do_call+0x13/0x13 > > > > > > > > > > Signed-off-by: Chuansheng Liu > > > > > Signed-off-by: Zhang Dongxing > > > > > Signed-off-by: xiaomin1 > > > > > --- > > > > > arch/mips/cavium-octeon/dma-octeon.c | 2 +- > > > > > arch/mips/netlogic/common/nlm-dma.c | 2 +- > > > > > drivers/xen/swiotlb-xen.c | 6 +++--- > > > > > include/linux/swiotlb.h | 8 +------ > > > > > lib/swiotlb.c | 39 ++++++++++++++++++++++++---------- > > > > > 5 files changed, 34 insertions(+), 23 deletions(-) > > > > > > > > > > diff --git a/arch/mips/cavium-octeon/dma-octeon.c > > > > > b/arch/mips/cavium-octeon/dma-octeon.c > > > > > index 3778655..a521af6 100644 > > > > > --- a/arch/mips/cavium-octeon/dma-octeon.c > > > > > +++ b/arch/mips/cavium-octeon/dma-octeon.c > > > > > @@ -312,7 +312,7 @@ void __init plat_swiotlb_setup(void) > > > > > swiotlbsize = 64 * (1<<20); > > > > > #endif > > > > > swiotlb_nslabs = swiotlbsize >> IO_TLB_SHIFT; > > > > > - swiotlb_nslabs = ALIGN(swiotlb_nslabs, IO_TLB_SEGSIZE); > > > > > + swiotlb_nslabs = ALIGN(swiotlb_nslabs, io_tlb_segsize); > > > > > swiotlbsize = swiotlb_nslabs << IO_TLB_SHIFT; > > > > > > > > > > octeon_swiotlb = alloc_bootmem_low_pages(swiotlbsize); > > > > > diff --git a/arch/mips/netlogic/common/nlm-dma.c > > > > > b/arch/mips/netlogic/common/nlm-dma.c > > > > > index f3d4ae8..eeffa8f 100644 > > > > > --- a/arch/mips/netlogic/common/nlm-dma.c > > > > > +++ b/arch/mips/netlogic/common/nlm-dma.c > > > > > @@ -99,7 +99,7 @@ void __init plat_swiotlb_setup(void) > > > > > > > > > > swiotlbsize = 1 << 20; /* 1 MB for now */ > > > > > swiotlb_nslabs = swiotlbsize >> IO_TLB_SHIFT; > > > > > - swiotlb_nslabs = ALIGN(swiotlb_nslabs, IO_TLB_SEGSIZE); > > > > > + swiotlb_nslabs = ALIGN(swiotlb_nslabs, io_tlb_segsize); > > > > > swiotlbsize = swiotlb_nslabs << IO_TLB_SHIFT; > > > > > > > > > > nlm_swiotlb = alloc_bootmem_low_pages(swiotlbsize); > > > > > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c > > > > > index 810ad41..3b3e9fe 100644 > > > > > --- a/drivers/xen/swiotlb-xen.c > > > > > +++ b/drivers/xen/swiotlb-xen.c > > > > > @@ -164,11 +164,11 @@ xen_swiotlb_fixup(void *buf, size_t size, > > > > unsigned long nslabs) > > > > > dma_addr_t dma_handle; > > > > > phys_addr_t p = virt_to_phys(buf); > > > > > > > > > > - dma_bits = get_order(IO_TLB_SEGSIZE << IO_TLB_SHIFT) + > > > > PAGE_SHIFT; > > > > > + dma_bits = get_order(io_tlb_segsize << IO_TLB_SHIFT) + > > > > > +PAGE_SHIFT; > > > > > > > > > > i = 0; > > > > > do { > > > > > - int slabs = min(nslabs - i, (unsigned long)IO_TLB_SEGSIZE); > > > > > + int slabs = min(nslabs - i, (unsigned long)io_tlb_segsize); > > > > > > > > > > do { > > > > > rc = xen_create_contiguous_region( @@ -187,7 > > > > +187,7 @@ static > > > > > unsigned long xen_set_nslabs(unsigned long nr_tbl) { > > > > > if (!nr_tbl) { > > > > > xen_io_tlb_nslabs = (64 * 1024 * 1024 >> IO_TLB_SHIFT); > > > > > - xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs, > > > > IO_TLB_SEGSIZE); > > > > > + xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs, io_tlb_segsize); > > > > > } else > > > > > xen_io_tlb_nslabs = nr_tbl; > > > > > > > > > > diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h > > > > > index e7a018e..13506db 100644 > > > > > --- a/include/linux/swiotlb.h > > > > > +++ b/include/linux/swiotlb.h > > > > > @@ -8,13 +8,7 @@ struct dma_attrs; struct scatterlist; > > > > > > > > > > extern int swiotlb_force; > > > > > - > > > > > -/* > > > > > - * Maximum allowable number of contiguous slabs to map, > > > > > - * must be a power of 2. What is the appropriate value ? > > > > > - * The complexity of {map,unmap}_single is linearly dependent on > > > > > this > > > > value. > > > > > - */ > > > > > -#define IO_TLB_SEGSIZE 128 > > > > > +extern int io_tlb_segsize; > > > > > > > > > > /* > > > > > * log of the size of each IO TLB slab. The number of slabs is > > > > > command line diff --git a/lib/swiotlb.c b/lib/swiotlb.c index > > > > > 4abda07..50c415a 100644 > > > > > --- a/lib/swiotlb.c > > > > > +++ b/lib/swiotlb.c > > > > > @@ -56,6 +56,15 @@ > > > > > int swiotlb_force; > > > > > > > > > > /* > > > > > + * Maximum allowable number of contiguous slabs to map, > > > > > + * must be a power of 2. What is the appropriate value ? > > > > > + * define io_tlb_segsize as a parameter > > > > > + * which can be changed dynamically in config file for special usage. > > > > > + * The complexity of {map,unmap}_single is linearly dependent on > > > > > + this > > > > value. > > > > > + */ > > > > > +int io_tlb_segsize = 128; > > > > > + > > > > > +/* > > > > > * Used to do a quick range check in swiotlb_tbl_unmap_single and > > > > > * swiotlb_tbl_sync_single_*, to see if the memory was in fact > > > > > allocated by > > > > this > > > > > * API. > > > > > @@ -97,12 +106,20 @@ static DEFINE_SPINLOCK(io_tlb_lock); static > > > > > int late_alloc; > > > > > > > > > > static int __init > > > > > +setup_io_tlb_segsize(char *str) > > > > > +{ > > > > > + get_option(&str, &io_tlb_segsize); > > > > > + return 0; > > > > > +} > > > > > +__setup("io_tlb_segsize=", setup_io_tlb_segsize); > > > > > + > > > > > +static int __init > > > > > setup_io_tlb_npages(char *str) > > > > > { > > > > > if (isdigit(*str)) { > > > > > io_tlb_nslabs = simple_strtoul(str, &str, 0); > > > > > - /* avoid tail segment of size < IO_TLB_SEGSIZE */ > > > > > - io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE); > > > > > + /* avoid tail segment of size < io_tlb_segsize */ > > > > > + io_tlb_nslabs = ALIGN(io_tlb_nslabs, io_tlb_segsize); > > > > > } > > > > > if (*str == ',') > > > > > ++str; > > > > > @@ -183,7 +200,7 @@ int __init swiotlb_init_with_tbl(char *tlb, > > > > > unsigned long nslabs, int verbose) > > > > > > > > > > /* > > > > > * Allocate and initialize the free list array. This array is used > > > > > - * to find contiguous free memory regions of size up to > > > > IO_TLB_SEGSIZE > > > > > + * to find contiguous free memory regions of size up to > > > > > +io_tlb_segsize > > > > > * between io_tlb_start and io_tlb_end. > > > > > */ > > > > > io_tlb_list = memblock_virt_alloc( @@ -193,7 +210,7 @@ int > > > > > __init swiotlb_init_with_tbl(char *tlb, unsigned > > > > long nslabs, int verbose) > > > > > PAGE_ALIGN(io_tlb_nslabs * > > > > sizeof(phys_addr_t)), > > > > > PAGE_SIZE); > > > > > for (i = 0; i < io_tlb_nslabs; i++) { > > > > > - io_tlb_list[i] = IO_TLB_SEGSIZE - OFFSET(i, IO_TLB_SEGSIZE); > > > > > + io_tlb_list[i] = io_tlb_segsize - OFFSET(i, io_tlb_segsize); > > > > > io_tlb_orig_addr[i] = INVALID_PHYS_ADDR; > > > > > } > > > > > io_tlb_index = 0; > > > > > @@ -217,7 +234,7 @@ swiotlb_init(int verbose) > > > > > > > > > > if (!io_tlb_nslabs) { > > > > > io_tlb_nslabs = (default_size >> IO_TLB_SHIFT); > > > > > - io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE); > > > > > + io_tlb_nslabs = ALIGN(io_tlb_nslabs, io_tlb_segsize); > > > > > } > > > > > > > > > > bytes = io_tlb_nslabs << IO_TLB_SHIFT; @@ -249,7 +266,7 @@ > > > > > swiotlb_late_init_with_default_size(size_t default_size) > > > > > > > > > > if (!io_tlb_nslabs) { > > > > > io_tlb_nslabs = (default_size >> IO_TLB_SHIFT); > > > > > - io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE); > > > > > + io_tlb_nslabs = ALIGN(io_tlb_nslabs, io_tlb_segsize); > > > > > } > > > > > > > > > > /* > > > > > @@ -308,7 +325,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned > > > > > long nslabs) > > > > > > > > > > /* > > > > > * Allocate and initialize the free list array. This array is used > > > > > - * to find contiguous free memory regions of size up to > > > > IO_TLB_SEGSIZE > > > > > + * to find contiguous free memory regions of size up to > > > > > +io_tlb_segsize > > > > > * between io_tlb_start and io_tlb_end. > > > > > */ > > > > > io_tlb_list = (unsigned int *)__get_free_pages(GFP_KERNEL, @@ - > > > > 324,7 > > > > > +341,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long > > > > > +nslabs) > > > > > goto cleanup4; > > > > > > > > > > for (i = 0; i < io_tlb_nslabs; i++) { > > > > > - io_tlb_list[i] = IO_TLB_SEGSIZE - OFFSET(i, IO_TLB_SEGSIZE); > > > > > + io_tlb_list[i] = io_tlb_segsize - OFFSET(i, io_tlb_segsize); > > > > > io_tlb_orig_addr[i] = INVALID_PHYS_ADDR; > > > > > } > > > > > io_tlb_index = 0; > > > > > @@ -493,7 +510,7 @@ phys_addr_t swiotlb_tbl_map_single(struct > > > > > device *hwdev, > > > > > > > > > > for (i = index; i < (int) (index + nslots); i++) > > > > > io_tlb_list[i] = 0; > > > > > - for (i = index - 1; (OFFSET(i, IO_TLB_SEGSIZE) != > > > > IO_TLB_SEGSIZE - 1) && io_tlb_list[i]; i--) > > > > > + for (i = index - 1; (OFFSET(i, io_tlb_segsize) != > > > > io_tlb_segsize - > > > > > +1) && io_tlb_list[i]; i--) > > > > > io_tlb_list[i] = ++count; > > > > > tlb_addr = io_tlb_start + (index << IO_TLB_SHIFT); > > > > > > > > > > @@ -571,7 +588,7 @@ void swiotlb_tbl_unmap_single(struct device > > > > *hwdev, phys_addr_t tlb_addr, > > > > > */ > > > > > spin_lock_irqsave(&io_tlb_lock, flags); > > > > > { > > > > > - count = ((index + nslots) < ALIGN(index + 1, IO_TLB_SEGSIZE) ? > > > > > + count = ((index + nslots) < ALIGN(index + 1, io_tlb_segsize) ? > > > > > io_tlb_list[index + nslots] : 0); > > > > > /* > > > > > * Step 1: return the slots to the free list, merging the @@ - > > > > 585,7 > > > > > +602,7 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, > > > > phys_addr_t tlb_addr, > > > > > * Step 2: merge the returned slots with the preceding slots, > > > > > * if available (non zero) > > > > > */ > > > > > - for (i = index - 1; (OFFSET(i, IO_TLB_SEGSIZE) != > > > > IO_TLB_SEGSIZE -1) && io_tlb_list[i]; i--) > > > > > + for (i = index - 1; (OFFSET(i, io_tlb_segsize) != > > > > > +io_tlb_segsize > > > > > +-1) && io_tlb_list[i]; i--) > > > > > io_tlb_list[i] = ++count; > > > > > } > > > > > spin_unlock_irqrestore(&io_tlb_lock, flags); > > > > > -- > > > > > 1.7.9.5 > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/