Re: Load increase after memory upgrade (part2)

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 1974 bytes --]

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 <konrad.wilk@oracle.com> 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

[-- Attachment #2: vmalloc --]
[-- Type: text/plain, Size: 3726 bytes --]

commit 0b5428f4a22be4855b5f03aa1369f9e30e095014
Author: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
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 <konrad.wilk@oracle.com>

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;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel