public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Brandon Philips <brandon@ifup.org>
To: mchehab@infradead.org
Cc: v4l-dvb-maintainer@linuxtv.org, video4linux-list@redhat.com
Subject: [PATCH 6 of 9] videobuf-vmalloc.c: Fix hack of postponing mmap on remap failure
Date: Fri, 28 Mar 2008 03:18:37 -0700	[thread overview]
Message-ID: <ab74ebf10c01d6a8a54a.1206699517@localhost> (raw)
In-Reply-To: <patchbomb.1206699511@localhost>

# HG changeset patch
# User Brandon Philips <brandon@ifup.org>
# Date 1206699280 25200
# Node ID ab74ebf10c01d6a8a54a39b6750bc86ec3e72286
# Parent  eb99bdd0a7e3f70eb40fcc6918794a8b8822ac49
videobuf-vmalloc.c: Fix hack of postponing mmap on remap failure

In videobuf-vmalloc.c remap_vmalloc_range is failing when applications are
trying to mmap buffers immediately after reqbuf.  It fails because the vmalloc
area isn't setup until the first QBUF when drivers call iolock.

This patch introduces mmap_setup to the qtype_ops and it is called in
__videobuf_mmap_setup if the buffer type is mmap.  In the case of vmalloc
buffers this calls iolock, and sets the state to idle.

I don't think this is needed for dma-sg buffers and it defaults to a no-op for
everything but vmalloc.

Signed-off-by: Brandon Philips <bphilips@suse.de>

---
 linux/drivers/media/video/videobuf-core.c    |   29 +++++++-----------
 linux/drivers/media/video/videobuf-vmalloc.c |   43 +++++++++++++--------------
 linux/include/media/videobuf-core.h          |    3 +
 3 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/linux/drivers/media/video/videobuf-core.c b/linux/drivers/media/video/videobuf-core.c
--- a/linux/drivers/media/video/videobuf-core.c
+++ b/linux/drivers/media/video/videobuf-core.c
@@ -92,24 +92,16 @@ int videobuf_iolock(struct videobuf_queu
 	MAGIC_CHECK(vb->magic, MAGIC_BUFFER);
 	MAGIC_CHECK(q->int_ops->magic, MAGIC_QTYPE_OPS);
 
-	/* This is required to avoid OOPS on some cases,
-	   since mmap_mapper() method should be called before _iolock.
-	   On some cases, the mmap_mapper() is called only after scheduling.
-	 */
-	if (vb->memory == V4L2_MEMORY_MMAP) {
-		wait_event_timeout(vb->done, q->is_mmapped,
-				   msecs_to_jiffies(100));
-		if (!q->is_mmapped) {
-			printk(KERN_ERR
-			       "Error: mmap_mapper() never called!\n");
-			return -EINVAL;
-		}
-	}
-
 	return CALL(q, iolock, q, vb, fbuf);
 }
 
 /* --------------------------------------------------------------------- */
+
+static int videobuf_mmap_setup_default(struct videobuf_queue *q,
+					struct videobuf_buffer *vb)
+{
+	return 0;
+}
 
 
 void videobuf_queue_core_init(struct videobuf_queue *q,
@@ -131,6 +123,9 @@ void videobuf_queue_core_init(struct vid
 	q->ops       = ops;
 	q->priv_data = priv;
 	q->int_ops   = int_ops;
+
+	if (!q->int_ops->mmap_setup)
+		q->int_ops->mmap_setup = videobuf_mmap_setup_default;
 
 	/* All buffer operations are mandatory */
 	BUG_ON(!q->ops->buf_setup);
@@ -305,8 +300,6 @@ static int __videobuf_mmap_free(struct v
 
 	rc  = CALL(q, mmap_free, q);
 
-	q->is_mmapped = 0;
-
 	if (rc < 0)
 		return rc;
 
@@ -358,6 +351,9 @@ static int __videobuf_mmap_setup(struct 
 		switch (memory) {
 		case V4L2_MEMORY_MMAP:
 			q->bufs[i]->boff  = bsize * i;
+			err = q->int_ops->mmap_setup(q, q->bufs[i]);
+			if (err)
+				break;
 			break;
 		case V4L2_MEMORY_USERPTR:
 		case V4L2_MEMORY_OVERLAY:
@@ -1018,7 +1014,6 @@ int videobuf_mmap_mapper(struct videobuf
 
 	mutex_lock(&q->vb_lock);
 	retval = CALL(q, mmap_mapper, q, vma);
-	q->is_mmapped = 1;
 	mutex_unlock(&q->vb_lock);
 
 	return retval;
diff --git a/linux/drivers/media/video/videobuf-vmalloc.c b/linux/drivers/media/video/videobuf-vmalloc.c
--- a/linux/drivers/media/video/videobuf-vmalloc.c
+++ b/linux/drivers/media/video/videobuf-vmalloc.c
@@ -152,21 +152,26 @@ static int __videobuf_iolock (struct vid
 				(unsigned long)mem->vmalloc,
 				pages << PAGE_SHIFT);
 
-	/* It seems that some kernel versions need to do remap *after*
-	   the mmap() call
-	 */
-	if (mem->vma) {
-		int retval=remap_vmalloc_range(mem->vma, mem->vmalloc,0);
-		kfree(mem->vma);
-		mem->vma=NULL;
-		if (retval<0) {
-			dprintk(1,"mmap app bug: remap_vmalloc_range area %p error %d\n",
-				mem->vmalloc,retval);
-			return retval;
+	return 0;
+}
+
+static int __videobuf_mmap_setup(struct videobuf_queue *q,
+			      struct videobuf_buffer *vb)
+{
+	int retval = 0;
+	BUG_ON(vb->memory != V4L2_MEMORY_MMAP);
+	if (vb->state == VIDEOBUF_NEEDS_INIT) {
+		/* bsize == size since the buffer needs to be large enough to
+		 * hold an entire frame, not the case in the read case for
+		 * example*/
+		vb->size = vb->bsize;
+		retval = __videobuf_iolock(q, vb, NULL);
+		if (!retval) {
+			/* Don't IOLOCK later */
+			vb->state = VIDEOBUF_IDLE;
 		}
 	}
-
-	return 0;
+	return retval;
 }
 
 static int __videobuf_sync(struct videobuf_queue *q,
@@ -239,15 +244,8 @@ static int __videobuf_mmap_mapper(struct
 	/* Try to remap memory */
 	retval=remap_vmalloc_range(vma, mem->vmalloc,0);
 	if (retval<0) {
-		dprintk(1,"mmap: postponing remap_vmalloc_range\n");
-
-		mem->vma=kmalloc(sizeof(*vma),GFP_KERNEL);
-		if (!mem->vma) {
-			kfree(map);
-			q->bufs[first]->map=NULL;
-			return -ENOMEM;
-		}
-		memcpy(mem->vma,vma,sizeof(*vma));
+		dprintk(1, "mmap: failed to remap_vmalloc_range\n");
+		return -EINVAL;
 	}
 
 	dprintk(1,"mmap %p: q=%p %08lx-%08lx (%lx) pgoff %08lx buf %d\n",
@@ -315,6 +313,7 @@ static struct videobuf_qtype_ops qops = 
 	.alloc        = __videobuf_alloc,
 	.iolock       = __videobuf_iolock,
 	.sync         = __videobuf_sync,
+	.mmap_setup   = __videobuf_mmap_setup,
 	.mmap_free    = __videobuf_mmap_free,
 	.mmap_mapper  = __videobuf_mmap_mapper,
 	.video_copy_to_user = __videobuf_copy_to_user,
diff --git a/linux/include/media/videobuf-core.h b/linux/include/media/videobuf-core.h
--- a/linux/include/media/videobuf-core.h
+++ b/linux/include/media/videobuf-core.h
@@ -144,6 +144,8 @@ struct videobuf_qtype_ops {
 				 int vbihack,
 				 int nonblocking);
 	int (*mmap_free)	(struct videobuf_queue *q);
+	int (*mmap_setup)	(struct videobuf_queue *q,
+				 struct videobuf_buffer *vb);
 	int (*mmap_mapper)	(struct videobuf_queue *q,
 				struct vm_area_struct *vma);
 };
@@ -168,7 +170,6 @@ struct videobuf_queue {
 
 	unsigned int               streaming:1;
 	unsigned int               reading:1;
-	unsigned int		   is_mmapped:1;
 
 	/* capture via mmap() + ioctl(QBUF/DQBUF) */
 	struct list_head           stream;

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

  parent reply	other threads:[~2008-03-28 10:34 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-28 10:18 [PATCH 0 of 9] videobuf fixes Brandon Philips
2008-03-28 10:18 ` [PATCH 1 of 9] soc_camera: Introduce a spinlock for use with videobuf Brandon Philips
2008-03-28 20:53   ` Guennadi Liakhovetski
2008-03-29  3:59     ` Brandon Philips
2008-04-04 13:46       ` [PATCH] soc-camera: use a spinlock for videobuffer queue Guennadi Liakhovetski
2008-04-04 20:17         ` Brandon Philips
2008-03-28 10:18 ` [PATCH 2 of 9] videobuf: Require spinlocks for all videobuf users Brandon Philips
2008-03-28 10:18 ` [PATCH 3 of 9] videobuf: Wakeup queues after changing the state to ERROR Brandon Philips
2008-03-28 10:18 ` [PATCH 4 of 9] videobuf: Simplify videobuf_waiton logic and possibly avoid missed wakeup Brandon Philips
2008-03-28 10:18 ` [PATCH 5 of 9] videobuf-vmalloc.c: Remove buf_release from videobuf_vm_close Brandon Philips
2008-03-28 10:18 ` Brandon Philips [this message]
     [not found]   ` <20080405131236.7c083554@gaivota>
     [not found]     ` <20080406080011.GA3596@plankton.ifup.org>
     [not found]       ` <20080407183226.703217fc@gaivota>
     [not found]         ` <20080408152238.GA8438@plankton.public.utexas.edu>
2008-04-08 18:40           ` [PATCH 6 of 9] videobuf-vmalloc.c: Fix hack of postponing mmap on remap failure Mauro Carvalho Chehab
     [not found]             ` <c8b4dbe10804081306xb1e8f91q64d1e6d18d3d2531@mail.gmail.com>
2008-04-08 20:50               ` Mauro Carvalho Chehab
     [not found]                 ` <c8b4dbe10804090626l6b2b10d9p1c22e02dfe2850fe@mail.gmail.com>
2008-04-09 20:42                   ` Mauro Carvalho Chehab
     [not found]             ` <20080408204514.GA6844@plankton.public.utexas.edu>
2008-04-08 21:37               ` Mauro Carvalho Chehab
     [not found]                 ` <20080415021558.GA22068@plankton.ifup.org>
2008-04-15 14:10                   ` Mauro Carvalho Chehab
2008-03-28 10:18 ` [PATCH 7 of 9] vivi: Simplify the vivi driver and avoid deadlocks Brandon Philips
2008-03-28 18:34   ` Mauro Carvalho Chehab
2008-03-29  5:35     ` Brandon Philips
2008-03-31 19:35       ` Mauro Carvalho Chehab
2008-03-28 10:18 ` [PATCH 8 of 9] videobuf: Avoid deadlock with QBUF and bring up to spec for empty queue Brandon Philips
2008-03-28 10:18 ` [PATCH 9 of 9] videobuf-dma-sg.c: Avoid NULL dereference and add comment about backwards compatibility Brandon Philips
2008-03-28 19:09 ` [PATCH 0 of 9] videobuf fixes Mauro Carvalho Chehab
2008-03-29  5:25   ` Brandon Philips
2008-03-29 22:49     ` Vanessa Ezekowitz
2008-03-31 18:35     ` Mauro Carvalho Chehab
2008-03-31 19:26       ` Brandon Philips
2008-03-31 21:31         ` Mauro Carvalho Chehab
2008-04-01  3:11           ` Brandon Philips
2008-04-01 20:49             ` Mauro Carvalho Chehab
2008-04-02 18:54               ` Brandon Philips
2008-04-02 20:06                 ` Mauro Carvalho Chehab
2008-04-02 18:56               ` Brandon Philips

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ab74ebf10c01d6a8a54a.1206699517@localhost \
    --to=brandon@ifup.org \
    --cc=mchehab@infradead.org \
    --cc=v4l-dvb-maintainer@linuxtv.org \
    --cc=video4linux-list@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox