Resend:videobuf_dma_sg: a new implementation for mmap

a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping when it encounter page fault at video_vm_ops->fault(). pls see
http://www.spinics.net/lists/linux-media/msg21243.html

a new implementation for mmap, it translate to vmalloc to page at video_vm_ops->fault().

in v2, if mem->dma.vmalloc is NULL at video_vm_ops->fault(), it will alloc memory by vmlloc_32().

Signed-off-by Figo.zhang <[email protected]>

---
drivers/media/video/videobuf-dma-sg.c | 50
+++++++++++++++++++++++++++------ 1 files changed, 41 insertions(+), 9 deletions(-)

(quoted diff) +0 -0

b/drivers/media/video/videobuf-dma-sg.c index 8359e6b..f7295da 100644

drivers/media/video/videobuf-dma-sg.c +41 -9

@@ -201,10 +201,11 @@ int videobuf_dma_init_kernel(struct videobuf_dmabuf
 *dma, int direction, dprintk(1, "init kernel [%d pages]\n", nr_pages);
 
 	dma->direction = direction;
-	dma->vmalloc = vmalloc_32(nr_pages << PAGE_SHIFT);
-	if (NULL == dma->vmalloc) {
-		dprintk(1, "vmalloc_32(%d pages) failed\n", nr_pages);
-		return -ENOMEM;
+	if (!dma->vmalloc)
+		dma->vmalloc = vmalloc_32(nr_pages << PAGE_SHIFT);
+		if (NULL == dma->vmalloc) {
+			dprintk(1, "vmalloc_32(%d pages) failed\n", nr_pages);
+			return -ENOMEM;
 	}
 
 	dprintk(1, "vmalloc is at addr 0x%08lx, size=%d\n",

@@ -397,16 +398,47 @@ static void videobuf_vm_close(struct vm_area_struct
 *vma) */
 static int videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault
 *vmf) {
-	struct page *page;
+	struct page *page = NULL;
+	struct videobuf_mapping *map = vma->vm_private_data;
+	struct videobuf_queue *q = map->q;
+	struct videobuf_dma_sg_memory *mem = NULL;
+
+	unsigned long offset;
+	unsigned long page_nr;
+	int first;
 
 	dprintk(3, "fault: fault @ %08lx [vma %08lx-%08lx]\n",
 		(unsigned long)vmf->virtual_address,
 		vma->vm_start, vma->vm_end);
 
-	page = alloc_page(GFP_USER | __GFP_DMA32);
-	if (!page)
-		return VM_FAULT_OOM;
-	clear_user_highpage(page, (unsigned long)vmf->virtual_address);
+	mutex_lock(&q->vb_lock);
+	offset = (unsigned long)vmf->virtual_address - vma->vm_start;
+	page_nr = offset >> PAGE_SHIFT;
+
+	for (first = 0; first < VIDEO_MAX_FRAME; first++) {
+			if (NULL == q->bufs[first])
+				continue;
+
+			MAGIC_CHECK(mem->magic, MAGIC_SG_MEM);
+
+			if (q->bufs[first]->map == map)
+				break;
+	}
+
+	mem = q->bufs[first]->priv;
+	if (!mem)
+		return VM_FAULT_SIGBUS;
+	if (!mem->dma.vmalloc) {
+		mem->dma.vmalloc = vmalloc_32(PAGE_ALIGN(q->bufs[first]->size));
+		if (NULL == mem->dma.vmalloc) {
+			dprintk(1, "%s: vmalloc_32() failed\n", __func__);
+			return VM_FAULT_OOM;
+		}
+	} else
+		page = vmalloc_to_page(mem->dma.vmalloc+
+				(offset & (~PAGE_MASK)));
+	mutex_unlock(&q->vb_lock);
+
 	vmf->page = page;
 
 	return 0;

Hi,

Quoted 7 lines · On Friday 30 July 2010 02:08:02 Figo.zhang wrote:

a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping when it encounter page fault at video_vm_ops->fault(). pls see
http://www.spinics.net/lists/linux-media/msg21243.html

a new implementation for mmap, it translate to vmalloc to page at video_vm_ops->fault().

Quoted 2 lines

in v2, if mem->dma.vmalloc is NULL at video_vm_ops->fault(), it will alloc memory by vmlloc_32().

You're replacing allocation in videobuf_vm_fault by allocationg in videobuf_vm_fault. I don't see the point. videobuf_vm_fault needs to go away completely.

This has been discussed previously: fixing videobuf is not really possible. A videobuf2 implementation is needed and is (slowly) being worked on.

I wouldn't
bother with this patch, just drop it.

Quoted 79 lines

Signed-off-by Figo.zhang <[email protected]>

---
drivers/media/video/videobuf-dma-sg.c | 50
+++++++++++++++++++++++++++------ 1 files changed, 41 insertions(+), 9 deletions(-)

(quoted diff) +0 -0

b/drivers/media/video/videobuf-dma-sg.c index 8359e6b..f7295da 100644

drivers/media/video/videobuf-dma-sg.c +41 -9

@@ -201,10 +201,11 @@ int videobuf_dma_init_kernel(struct videobuf_dmabuf
 *dma, int direction, dprintk(1, "init kernel [%d pages]\n", nr_pages);
 
 	dma->direction = direction;
-	dma->vmalloc = vmalloc_32(nr_pages << PAGE_SHIFT);
-	if (NULL == dma->vmalloc) {
-		dprintk(1, "vmalloc_32(%d pages) failed\n", nr_pages);
-		return -ENOMEM;
+	if (!dma->vmalloc)
+		dma->vmalloc = vmalloc_32(nr_pages << PAGE_SHIFT);
+		if (NULL == dma->vmalloc) {
+			dprintk(1, "vmalloc_32(%d pages) failed\n", nr_pages);
+			return -ENOMEM;
 	}
 
 	dprintk(1, "vmalloc is at addr 0x%08lx, size=%d\n",

@@ -397,16 +398,47 @@ static void videobuf_vm_close(struct vm_area_struct
 *vma) */
 static int videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault
 *vmf) {
-	struct page *page;
+	struct page *page = NULL;
+	struct videobuf_mapping *map = vma->vm_private_data;
+	struct videobuf_queue *q = map->q;
+	struct videobuf_dma_sg_memory *mem = NULL;
+
+	unsigned long offset;
+	unsigned long page_nr;
+	int first;
 
 	dprintk(3, "fault: fault @ %08lx [vma %08lx-%08lx]\n",
 		(unsigned long)vmf->virtual_address,
 		vma->vm_start, vma->vm_end);
 
-	page = alloc_page(GFP_USER | __GFP_DMA32);
-	if (!page)
-		return VM_FAULT_OOM;
-	clear_user_highpage(page, (unsigned long)vmf->virtual_address);
+	mutex_lock(&q->vb_lock);
+	offset = (unsigned long)vmf->virtual_address - vma->vm_start;
+	page_nr = offset >> PAGE_SHIFT;
+
+	for (first = 0; first < VIDEO_MAX_FRAME; first++) {
+			if (NULL == q->bufs[first])
+				continue;
+
+			MAGIC_CHECK(mem->magic, MAGIC_SG_MEM);
+
+			if (q->bufs[first]->map == map)
+				break;
+	}
+
+	mem = q->bufs[first]->priv;
+	if (!mem)
+		return VM_FAULT_SIGBUS;
+	if (!mem->dma.vmalloc) {
+		mem->dma.vmalloc = vmalloc_32(PAGE_ALIGN(q->bufs[first]->size));
+		if (NULL == mem->dma.vmalloc) {
+			dprintk(1, "%s: vmalloc_32() failed\n", __func__);
+			return VM_FAULT_OOM;
+		}
+	} else
+		page = vmalloc_to_page(mem->dma.vmalloc+
+				(offset & (~PAGE_MASK)));
+	mutex_unlock(&q->vb_lock);
+
 	vmf->page = page;
 
 	return 0;

Hi,

Quoted 7 lines · On Friday 30 July 2010 02:08:02 Figo.zhang wrote:

a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping when it encounter page fault at video_vm_ops->fault(). pls see
http://www.spinics.net/lists/linux-media/msg21243.html

a new implementation for mmap, it translate to vmalloc to page at video_vm_ops->fault().

Quoted 2 lines

in v2, if mem->dma.vmalloc is NULL at video_vm_ops->fault(), it will alloc memory by vmlloc_32().

You're replacing allocation in videobuf_vm_fault by allocationg in videobuf_vm_fault. I don't see the point. videobuf_vm_fault needs to go away completely.

This has been discussed previously: fixing videobuf is not really possible. A videobuf2 implementation is needed and is (slowly) being worked on.

I wouldn't
bother with this patch, just drop it.

Quoted 79 lines

Signed-off-by Figo.zhang <[email protected]>

---
drivers/media/video/videobuf-dma-sg.c | 50
+++++++++++++++++++++++++++------ 1 files changed, 41 insertions(+), 9 deletions(-)

(quoted diff) +0 -0

b/drivers/media/video/videobuf-dma-sg.c index 8359e6b..f7295da 100644

drivers/media/video/videobuf-dma-sg.c +41 -9

@@ -201,10 +201,11 @@ int videobuf_dma_init_kernel(struct videobuf_dmabuf
 *dma, int direction, dprintk(1, "init kernel [%d pages]\n", nr_pages);
 
 	dma->direction = direction;
-	dma->vmalloc = vmalloc_32(nr_pages << PAGE_SHIFT);
-	if (NULL == dma->vmalloc) {
-		dprintk(1, "vmalloc_32(%d pages) failed\n", nr_pages);
-		return -ENOMEM;
+	if (!dma->vmalloc)
+		dma->vmalloc = vmalloc_32(nr_pages << PAGE_SHIFT);
+		if (NULL == dma->vmalloc) {
+			dprintk(1, "vmalloc_32(%d pages) failed\n", nr_pages);
+			return -ENOMEM;
 	}
 
 	dprintk(1, "vmalloc is at addr 0x%08lx, size=%d\n",

@@ -397,16 +398,47 @@ static void videobuf_vm_close(struct vm_area_struct
 *vma) */
 static int videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault
 *vmf) {
-	struct page *page;
+	struct page *page = NULL;
+	struct videobuf_mapping *map = vma->vm_private_data;
+	struct videobuf_queue *q = map->q;
+	struct videobuf_dma_sg_memory *mem = NULL;
+
+	unsigned long offset;
+	unsigned long page_nr;
+	int first;
 
 	dprintk(3, "fault: fault @ %08lx [vma %08lx-%08lx]\n",
 		(unsigned long)vmf->virtual_address,
 		vma->vm_start, vma->vm_end);
 
-	page = alloc_page(GFP_USER | __GFP_DMA32);
-	if (!page)
-		return VM_FAULT_OOM;
-	clear_user_highpage(page, (unsigned long)vmf->virtual_address);
+	mutex_lock(&q->vb_lock);
+	offset = (unsigned long)vmf->virtual_address - vma->vm_start;
+	page_nr = offset >> PAGE_SHIFT;
+
+	for (first = 0; first < VIDEO_MAX_FRAME; first++) {
+			if (NULL == q->bufs[first])
+				continue;
+
+			MAGIC_CHECK(mem->magic, MAGIC_SG_MEM);
+
+			if (q->bufs[first]->map == map)
+				break;
+	}
+
+	mem = q->bufs[first]->priv;
+	if (!mem)
+		return VM_FAULT_SIGBUS;
+	if (!mem->dma.vmalloc) {
+		mem->dma.vmalloc = vmalloc_32(PAGE_ALIGN(q->bufs[first]->size));
+		if (NULL == mem->dma.vmalloc) {
+			dprintk(1, "%s: vmalloc_32() failed\n", __func__);
+			return VM_FAULT_OOM;
+		}
+	} else
+		page = vmalloc_to_page(mem->dma.vmalloc+
+				(offset & (~PAGE_MASK)));
+	mutex_unlock(&q->vb_lock);
+
 	vmf->page = page;
 
 	return 0;

Quoted 13 lines · Laurent Pinchart

Hi,

Quoted 9 lines · On Friday 30 July 2010 02:08:02 Figo.zhang wrote:

a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping when it encounter page fault at video_vm_ops->fault(). pls see
http://www.spinics.net/lists/linux-media/msg21243.html

a new implementation for mmap, it translate to vmalloc to page at video_vm_ops->fault().

in v2, if mem->dma.vmalloc is NULL at video_vm_ops->fault(), it will alloc memory by vmlloc_32().

Quoted 3 lines

You're replacing allocation in videobuf_vm_fault by allocationg in videobuf_vm_fault. I don't see the point. videobuf_vm_fault needs to go away completely.

in now videobuf code, the mmap alloc a new buf, the capture dma buffer using vmalloc() alloc buffer, how is
relationship with them? in usrspace , the mmap region will not the actual capture dma data, how is work?