From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: Load increase after memory upgrade (part2) Date: Mon, 23 Jan 2012 17:32:13 -0500 Message-ID: <20120123223213.GA31929@phenom.dumpdata.com> References: <783969451.20111218011916@eikelenboom.it> <20111219145609.GB28969@andromeda.dapyr.net> <20120110215533.GA21862@phenom.dumpdata.com> <1442969761.20120112230601@eikelenboom.it> <20120113151307.GC5025@phenom.dumpdata.com> <1383590207.20120115123259@eikelenboom.it> <20120117210225.GA23782@phenom.dumpdata.com> <4F16BC97020000780006D6D6@nat28.tlf.novell.com> <20120118142923.GA6052@andromeda.dapyr.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="a8Wt8u1KmwUX3Y2C" Return-path: Content-Disposition: inline In-Reply-To: <20120118142923.GA6052@andromeda.dapyr.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Konrad Rzeszutek Wilk Cc: Sander Eikelenboom , xen-devel , Jan Beulich List-Id: xen-devel@lists.xenproject.org --a8Wt8u1KmwUX3Y2C Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Jan 18, 2012 at 10:29:23AM -0400, Konrad Rzeszutek Wilk wrote: > On Wed, Jan 18, 2012 at 11:35:35AM +0000, Jan Beulich wrote: > > >>> On 17.01.12 at 22:02, Konrad Rzeszutek Wilk wrote: > > > The issue as I understand is that the DVB drivers allocate their buffers > > > from 0->4GB most (all the time?) so they never have to do bounce-buffering. > > > > > > While the pv-ops one ends up quite frequently doing the bounce-buffering, > > > which > > > implies that the DVB drivers end up allocating their buffers above the 4GB. > > > This means we end up spending some CPU time (in the guest) copying the > > > memory > > > from >4GB to 0-4GB region (And vice-versa). > > > > This reminds me of something (not sure what XenoLinux you use for > > comparison) - how are they allocating that memory? Not vmalloc_32() > > I was using the 2.6.18, then the one I saw on Google for Gentoo, and now > I am going to look at the 2.6.38 from OpenSuSE. > > > by chance (I remember having seen numerous uses under - iirc - > > drivers/media/)? > > > > Obviously, vmalloc_32() and any GFP_DMA32 allocations do *not* do > > what their (driver) callers might expect in a PV guest (including the > > contiguity assumption for the latter, recalling that you earlier said > > you were able to see the problem after several guest starts), and I > > had put into our kernels an adjustment to make vmalloc_32() actually > > behave as expected. > > Aaah.. The plot thickens! Let me look in the sources! Thanks for the > pointer. Jan hints lead me to the videobuf-dma-sg.c which does indeed to vmalloc_32 and then performs PCI DMA operations on the allocted vmalloc_32 area. So I cobbled up the attached patch (hadn't actually tested it and sadly won't until next week) which removes the call to vmalloc_32 and instead sets up DMA allocated set of pages. If that fixes it for you that is awesome, but if it breaks please send me your logs. Cheers, Konrad --a8Wt8u1KmwUX3Y2C Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=vmalloc commit 0b5428f4a22be4855b5f03aa1369f9e30e095014 Author: Konrad Rzeszutek Wilk Date: Mon Jan 23 15:52:01 2012 -0500 vmalloc_sg: make sure all pages in vmalloc area are really DMA-ready Under Xen, vmalloc_32() isn't guaranteed to return pages which are really under 4G in machine physical addresses (only in virtual pseudo-physical addresses). To work around this, implement a vmalloc variant which allocates each page with dma_alloc_coherent() to guarantee that each page is suitable for the device in question. Signed-off-by: Konrad Rzeszutek Wilk diff --git a/drivers/media/video/videobuf-dma-sg.c b/drivers/media/video/videobuf-dma-sg.c index f300dea..3da2428 100644 --- a/drivers/media/video/videobuf-dma-sg.c +++ b/drivers/media/video/videobuf-dma-sg.c @@ -211,13 +211,36 @@ EXPORT_SYMBOL_GPL(videobuf_dma_init_user); int videobuf_dma_init_kernel(struct videobuf_dmabuf *dma, int direction, int nr_pages) { + int i; + dprintk(1, "init kernel [%d pages]\n", nr_pages); dma->direction = direction; - dma->vaddr = vmalloc_32(nr_pages << PAGE_SHIFT); + dma->vaddr_pages = kcalloc(nr_pages, sizeof(*dma->vaddr_pages), + GFP_KERNEL); + if (!dma->vaddr_pages) + return -ENOMEM; + + dma->dma_addr = kcalloc(nr_pages, sizeof(*dma->dma_addr), GFP_KERNEL); + if (!dma->dma_addr) { + kfree(dma->vaddr_pages); + return -ENOMEM; + } + for (i = 0; i < nr_pages; i++) { + void *addr; + + addr = dma_alloc_coherent(dma->dev, PAGE_SIZE, + &(dma->dma_addr[i]), GFP_KERNEL); + if (addr == NULL) + goto out_free_pages; + + dma->vaddr_pages[i] = virt_to_page(addr); + } + dma->vaddr = vmap(dma->vaddr_pages, nr_pages, VM_MAP | VM_IOREMAP, + PAGE_KERNEL); if (NULL == dma->vaddr) { dprintk(1, "vmalloc_32(%d pages) failed\n", nr_pages); - return -ENOMEM; + goto out_free_pages; } dprintk(1, "vmalloc is at addr 0x%08lx, size=%d\n", @@ -228,6 +251,18 @@ int videobuf_dma_init_kernel(struct videobuf_dmabuf *dma, int direction, dma->nr_pages = nr_pages; return 0; +out_free_pages: + while (i > 0) { + void *addr = page_address(dma->vaddr_pages[i]); + dma_free_coherent(dma->dev, PAGE_SIZE, addr, dma->dma_addr[i]); + i--; + } + kfree(dma->dma_addr); + dma->dma_addr = NULL; + kfree(dma->vaddr_pages); + dma->vaddr_pages = NULL; + + return -ENOMEM; } EXPORT_SYMBOL_GPL(videobuf_dma_init_kernel); @@ -322,8 +357,21 @@ int videobuf_dma_free(struct videobuf_dmabuf *dma) dma->pages = NULL; } - vfree(dma->vaddr); - dma->vaddr = NULL; + if (dma->dma_addr) { + for (i = 0; i < dma->nr_pages; i++) { + void *addr; + + addr = page_address(dma->vaddr_pages[i]); + dma_free_coherent(dma->dev, PAGE_SIZE, addr, + dma->dma_addr[i]); + } + kfree(dma->dma_addr); + dma->dma_addr = NULL; + kfree(dma->vaddr_pages); + dma->vaddr_pages = NULL; + vunmap(dma->vaddr); + dma->vaddr = NULL; + } if (dma->bus_addr) dma->bus_addr = 0; @@ -461,6 +509,11 @@ static int __videobuf_iolock(struct videobuf_queue *q, MAGIC_CHECK(mem->magic, MAGIC_SG_MEM); + if (!mem->dma.dev) + mem->dma.dev = q->dev; + else + WARN_ON(mem->dma.dev != q->dev); + switch (vb->memory) { case V4L2_MEMORY_MMAP: case V4L2_MEMORY_USERPTR: diff --git a/include/media/videobuf-dma-sg.h b/include/media/videobuf-dma-sg.h index d8fb601..870cb21 100644 --- a/include/media/videobuf-dma-sg.h +++ b/include/media/videobuf-dma-sg.h @@ -53,6 +53,9 @@ struct videobuf_dmabuf { /* for kernel buffers */ void *vaddr; + struct page **vaddr_pages; + dma_addr_t *dma_addr; + struct device *dev; /* for overlay buffers (pci-pci dma) */ dma_addr_t bus_addr; --a8Wt8u1KmwUX3Y2C Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --a8Wt8u1KmwUX3Y2C--