* [PATCHv3] videobuf2: fix lockdep warning
@ 2014-08-05 10:56 Hans Verkuil
2014-08-05 11:43 ` Marek Szyprowski
2014-08-05 22:15 ` Laurent Pinchart
0 siblings, 2 replies; 5+ messages in thread
From: Hans Verkuil @ 2014-08-05 10:56 UTC (permalink / raw)
To: linux-media, Laurent Pinchart, Pawel Osciak, Marek Szyprowski,
sasha.levin
Changes since v2: use a mutex instead of a spinlock due to possible sleeps inside
the mmap memop. Use the mutex when buffers are allocated/freed, so it's a much coarser
lock.
The following lockdep warning has been there ever since commit a517cca6b24fc54ac209e44118ec8962051662e3
one year ago:
[ 403.117947] ======================================================
[ 403.117949] [ INFO: possible circular locking dependency detected ]
[ 403.117953] 3.16.0-rc6-test-media #961 Not tainted
[ 403.117954] -------------------------------------------------------
[ 403.117956] v4l2-ctl/15377 is trying to acquire lock:
[ 403.117959] (&dev->mutex#3){+.+.+.}, at: [<ffffffffa005a6c3>] vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[ 403.117974]
[ 403.117974] but task is already holding lock:
[ 403.117976] (&mm->mmap_sem){++++++}, at: [<ffffffff8118291f>] vm_mmap_pgoff+0x6f/0xc0
[ 403.117987]
[ 403.117987] which lock already depends on the new lock.
[ 403.117987]
[ 403.117990]
[ 403.117990] the existing dependency chain (in reverse order) is:
[ 403.117992]
[ 403.117992] -> #1 (&mm->mmap_sem){++++++}:
[ 403.117997] [<ffffffff810d733c>] validate_chain.isra.39+0x5fc/0x9a0
[ 403.118006] [<ffffffff810d8bc3>] __lock_acquire+0x4d3/0xd30
[ 403.118010] [<ffffffff810d9da7>] lock_acquire+0xa7/0x160
[ 403.118014] [<ffffffff8118c9ec>] might_fault+0x7c/0xb0
[ 403.118018] [<ffffffffa0028a25>] video_usercopy+0x425/0x610 [videodev]
[ 403.118028] [<ffffffffa0028c25>] video_ioctl2+0x15/0x20 [videodev]
[ 403.118034] [<ffffffffa0022764>] v4l2_ioctl+0x184/0x1a0 [videodev]
[ 403.118040] [<ffffffff811d77d0>] do_vfs_ioctl+0x2f0/0x4f0
[ 403.118307] [<ffffffff811d7a51>] SyS_ioctl+0x81/0xa0
[ 403.118311] [<ffffffff8199dc69>] system_call_fastpath+0x16/0x1b
[ 403.118319]
[ 403.118319] -> #0 (&dev->mutex#3){+.+.+.}:
[ 403.118324] [<ffffffff810d6a96>] check_prevs_add+0x746/0x9f0
[ 403.118329] [<ffffffff810d733c>] validate_chain.isra.39+0x5fc/0x9a0
[ 403.118333] [<ffffffff810d8bc3>] __lock_acquire+0x4d3/0xd30
[ 403.118336] [<ffffffff810d9da7>] lock_acquire+0xa7/0x160
[ 403.118340] [<ffffffff81999664>] mutex_lock_interruptible_nested+0x64/0x640
[ 403.118344] [<ffffffffa005a6c3>] vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[ 403.118349] [<ffffffffa0022122>] v4l2_mmap+0x62/0xa0 [videodev]
[ 403.118354] [<ffffffff81197270>] mmap_region+0x3d0/0x5d0
[ 403.118359] [<ffffffff8119778d>] do_mmap_pgoff+0x31d/0x400
[ 403.118363] [<ffffffff81182940>] vm_mmap_pgoff+0x90/0xc0
[ 403.118366] [<ffffffff81195cef>] SyS_mmap_pgoff+0x1df/0x2a0
[ 403.118369] [<ffffffff810085c2>] SyS_mmap+0x22/0x30
[ 403.118376] [<ffffffff8199dc69>] system_call_fastpath+0x16/0x1b
[ 403.118381]
[ 403.118381] other info that might help us debug this:
[ 403.118381]
[ 403.118383] Possible unsafe locking scenario:
[ 403.118383]
[ 403.118385] CPU0 CPU1
[ 403.118387] ---- ----
[ 403.118388] lock(&mm->mmap_sem);
[ 403.118391] lock(&dev->mutex#3);
[ 403.118394] lock(&mm->mmap_sem);
[ 403.118397] lock(&dev->mutex#3);
[ 403.118400]
[ 403.118400] *** DEADLOCK ***
[ 403.118400]
[ 403.118403] 1 lock held by v4l2-ctl/15377:
[ 403.118405] #0: (&mm->mmap_sem){++++++}, at: [<ffffffff8118291f>] vm_mmap_pgoff+0x6f/0xc0
[ 403.118411]
[ 403.118411] stack backtrace:
[ 403.118415] CPU: 0 PID: 15377 Comm: v4l2-ctl Not tainted 3.16.0-rc6-test-media #961
[ 403.118418] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/31/2013
[ 403.118420] ffffffff82a6c9d0 ffff8800af37fb00 ffffffff819916a2 ffffffff82a6c9d0
[ 403.118425] ffff8800af37fb40 ffffffff810d5715 ffff8802308e4200 0000000000000000
[ 403.118429] ffff8802308e4a48 ffff8802308e4a48 ffff8802308e4200 0000000000000001
[ 403.118433] Call Trace:
[ 403.118441] [<ffffffff819916a2>] dump_stack+0x4e/0x7a
[ 403.118445] [<ffffffff810d5715>] print_circular_bug+0x1d5/0x2a0
[ 403.118449] [<ffffffff810d6a96>] check_prevs_add+0x746/0x9f0
[ 403.118455] [<ffffffff8119c172>] ? find_vmap_area+0x42/0x70
[ 403.118459] [<ffffffff810d733c>] validate_chain.isra.39+0x5fc/0x9a0
[ 403.118463] [<ffffffff810d8bc3>] __lock_acquire+0x4d3/0xd30
[ 403.118468] [<ffffffff810d9da7>] lock_acquire+0xa7/0x160
[ 403.118472] [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[ 403.118476] [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[ 403.118480] [<ffffffff81999664>] mutex_lock_interruptible_nested+0x64/0x640
[ 403.118484] [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[ 403.118488] [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[ 403.118493] [<ffffffff810d8055>] ? mark_held_locks+0x75/0xa0
[ 403.118497] [<ffffffffa005a6c3>] vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[ 403.118502] [<ffffffffa0022122>] v4l2_mmap+0x62/0xa0 [videodev]
[ 403.118506] [<ffffffff81197270>] mmap_region+0x3d0/0x5d0
[ 403.118510] [<ffffffff8119778d>] do_mmap_pgoff+0x31d/0x400
[ 403.118513] [<ffffffff81182940>] vm_mmap_pgoff+0x90/0xc0
[ 403.118517] [<ffffffff81195cef>] SyS_mmap_pgoff+0x1df/0x2a0
[ 403.118521] [<ffffffff810085c2>] SyS_mmap+0x22/0x30
[ 403.118525] [<ffffffff8199dc69>] system_call_fastpath+0x16/0x1b
The reason is that vb2_fop_mmap and vb2_fop_get_unmapped_area take the core lock
while they are called with the mmap_sem semaphore held. But elsewhere in the code
the core lock is taken first but calls to copy_to/from_user() can take the mmap_sem
semaphore as well, potentially causing a classical A-B/B-A deadlock.
However, the mmap/get_unmapped_area calls really shouldn't take the core lock
at all. So what would happen if they don't take the core lock anymore?
There are two situations that need to be taken into account: calling mmap while
new buffers are being added and calling mmap while buffers are being deleted.
The first case works almost fine without a lock: in all cases mmap relies on
correctly filled-in q->num_buffers/q->num_planes values and those are only
updated by reqbufs and create_buffers *after* any new buffers have been
initialized completely. Except in one case: if an error occurred while allocating
the buffers it will increase num_buffers and rely on __vb2_queue_free to
decrease it again. So there is a short period where the buffer information
may be wrong.
The second case definitely does pose a problem: buffers may be in the process
of being deleted, without the internal structure being updated.
In order to fix this a new mutex is added to vb2_queue that is taken when
buffers are allocated or deleted, and in vb2_mmap. That way vb2_mmap won't
get stale buffer data. Note that this is a problem only for MEMORY_MMAP, so
even though __qbuf_userptr and __qbuf_dmabuf also mess around with buffers
(mem_priv in particular), this doesn't clash with vb2_mmap or
vb2_get_unmapped_area since those are MMAP specific.
As an additional bonus the hack in __buf_prepare, the USERPTR case, can be
removed as well since mmap() no longer takes the core lock.
All in all a much cleaner solution.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/v4l2-core/videobuf2-core.c | 56 +++++++++++---------------------
include/media/videobuf2-core.h | 2 ++
2 files changed, 21 insertions(+), 37 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index c359006..6554bf3 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -882,7 +882,9 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
* We already have buffers allocated, so first check if they
* are not in use and can be freed.
*/
+ mutex_lock(&q->queue_lock);
if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
+ mutex_unlock(&q->queue_lock);
dprintk(1, "memory in use, cannot free\n");
return -EBUSY;
}
@@ -894,6 +896,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
*/
__vb2_queue_cancel(q);
ret = __vb2_queue_free(q, q->num_buffers);
+ mutex_unlock(&q->queue_lock);
if (ret)
return ret;
@@ -955,6 +958,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
*/
}
+ mutex_lock(&q->queue_lock);
q->num_buffers = allocated_buffers;
if (ret < 0) {
@@ -963,8 +967,10 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
* from q->num_buffers.
*/
__vb2_queue_free(q, allocated_buffers);
+ mutex_unlock(&q->queue_lock);
return ret;
}
+ mutex_unlock(&q->queue_lock);
/*
* Return the number of successfully allocated buffers
@@ -1061,6 +1067,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
*/
}
+ mutex_lock(&q->queue_lock);
q->num_buffers += allocated_buffers;
if (ret < 0) {
@@ -1069,8 +1076,10 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
* from q->num_buffers.
*/
__vb2_queue_free(q, allocated_buffers);
+ mutex_unlock(&q->queue_lock);
return -ENOMEM;
}
+ mutex_unlock(&q->queue_lock);
/*
* Return the number of successfully allocated buffers
@@ -1582,7 +1591,6 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
{
struct vb2_queue *q = vb->vb2_queue;
- struct rw_semaphore *mmap_sem;
int ret;
ret = __verify_length(vb, b);
@@ -1619,26 +1627,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
ret = __qbuf_mmap(vb, b);
break;
case V4L2_MEMORY_USERPTR:
- /*
- * In case of user pointer buffers vb2 allocators need to get
- * direct access to userspace pages. This requires getting
- * the mmap semaphore for read access in the current process
- * structure. The same semaphore is taken before calling mmap
- * operation, while both qbuf/prepare_buf and mmap are called
- * by the driver or v4l2 core with the driver's lock held.
- * To avoid an AB-BA deadlock (mmap_sem then driver's lock in
- * mmap and driver's lock then mmap_sem in qbuf/prepare_buf),
- * the videobuf2 core releases the driver's lock, takes
- * mmap_sem and then takes the driver's lock again.
- */
- mmap_sem = ¤t->mm->mmap_sem;
- call_void_qop(q, wait_prepare, q);
- down_read(mmap_sem);
- call_void_qop(q, wait_finish, q);
-
ret = __qbuf_userptr(vb, b);
-
- up_read(mmap_sem);
break;
case V4L2_MEMORY_DMABUF:
ret = __qbuf_dmabuf(vb, b);
@@ -2485,7 +2474,9 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
return -EINVAL;
}
+ mutex_lock(&q->queue_lock);
ret = call_memop(vb, mmap, vb->planes[plane].mem_priv, vma);
+ mutex_unlock(&q->queue_lock);
if (ret)
return ret;
@@ -2504,6 +2495,7 @@ unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
unsigned long off = pgoff << PAGE_SHIFT;
struct vb2_buffer *vb;
unsigned int buffer, plane;
+ void *vaddr;
int ret;
if (q->memory != V4L2_MEMORY_MMAP) {
@@ -2520,7 +2512,8 @@ unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
vb = q->bufs[buffer];
- return (unsigned long)vb2_plane_vaddr(vb, plane);
+ vaddr = vb2_plane_vaddr(vb, plane);
+ return vaddr ? (unsigned long)vaddr : -EINVAL;
}
EXPORT_SYMBOL_GPL(vb2_get_unmapped_area);
#endif
@@ -2660,6 +2653,7 @@ int vb2_queue_init(struct vb2_queue *q)
INIT_LIST_HEAD(&q->queued_list);
INIT_LIST_HEAD(&q->done_list);
spin_lock_init(&q->done_lock);
+ mutex_init(&q->queue_lock);
init_waitqueue_head(&q->done_wq);
if (q->buf_struct_size == 0)
@@ -2681,7 +2675,9 @@ void vb2_queue_release(struct vb2_queue *q)
{
__vb2_cleanup_fileio(q);
__vb2_queue_cancel(q);
+ mutex_lock(&q->queue_lock);
__vb2_queue_free(q, q->num_buffers);
+ mutex_unlock(&q->queue_lock);
}
EXPORT_SYMBOL_GPL(vb2_queue_release);
@@ -3346,15 +3342,8 @@ EXPORT_SYMBOL_GPL(vb2_ioctl_expbuf);
int vb2_fop_mmap(struct file *file, struct vm_area_struct *vma)
{
struct video_device *vdev = video_devdata(file);
- struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock;
- int err;
- if (lock && mutex_lock_interruptible(lock))
- return -ERESTARTSYS;
- err = vb2_mmap(vdev->queue, vma);
- if (lock)
- mutex_unlock(lock);
- return err;
+ return vb2_mmap(vdev->queue, vma);
}
EXPORT_SYMBOL_GPL(vb2_fop_mmap);
@@ -3473,15 +3462,8 @@ unsigned long vb2_fop_get_unmapped_area(struct file *file, unsigned long addr,
unsigned long len, unsigned long pgoff, unsigned long flags)
{
struct video_device *vdev = video_devdata(file);
- struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock;
- int ret;
- if (lock && mutex_lock_interruptible(lock))
- return -ERESTARTSYS;
- ret = vb2_get_unmapped_area(vdev->queue, addr, len, pgoff, flags);
- if (lock)
- mutex_unlock(lock);
- return ret;
+ return vb2_get_unmapped_area(vdev->queue, addr, len, pgoff, flags);
}
EXPORT_SYMBOL_GPL(vb2_fop_get_unmapped_area);
#endif
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index fc910a6..ae1289b 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -366,6 +366,7 @@ struct v4l2_fh;
* cannot be started unless at least this number of buffers
* have been queued into the driver.
*
+ * @queue_lock: private mutex used when buffers are allocated/freed/mmapped
* @memory: current memory type used
* @bufs: videobuf buffer structures
* @num_buffers: number of allocated/used buffers
@@ -399,6 +400,7 @@ struct vb2_queue {
u32 min_buffers_needed;
/* private: internal use only */
+ struct mutex queue_lock;
enum v4l2_memory memory;
struct vb2_buffer *bufs[VIDEO_MAX_FRAME];
unsigned int num_buffers;
--
2.0.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCHv3] videobuf2: fix lockdep warning
2014-08-05 10:56 [PATCHv3] videobuf2: fix lockdep warning Hans Verkuil
@ 2014-08-05 11:43 ` Marek Szyprowski
2014-08-05 22:15 ` Laurent Pinchart
1 sibling, 0 replies; 5+ messages in thread
From: Marek Szyprowski @ 2014-08-05 11:43 UTC (permalink / raw)
To: Hans Verkuil, linux-media, Laurent Pinchart, Pawel Osciak,
sasha.levin
Hello,
On 2014-08-05 12:56, Hans Verkuil wrote:
> Changes since v2: use a mutex instead of a spinlock due to possible sleeps inside
> the mmap memop. Use the mutex when buffers are allocated/freed, so it's a much coarser
> lock.
>
> The following lockdep warning has been there ever since commit a517cca6b24fc54ac209e44118ec8962051662e3
> one year ago:
>
> [ 403.117947] ======================================================
> [ 403.117949] [ INFO: possible circular locking dependency detected ]
> [ 403.117953] 3.16.0-rc6-test-media #961 Not tainted
> [ 403.117954] -------------------------------------------------------
> [ 403.117956] v4l2-ctl/15377 is trying to acquire lock:
> [ 403.117959] (&dev->mutex#3){+.+.+.}, at: [<ffffffffa005a6c3>] vb2_fop_mmap+0x33/0x90 [videobuf2_core]
> [ 403.117974]
> [ 403.117974] but task is already holding lock:
> [ 403.117976] (&mm->mmap_sem){++++++}, at: [<ffffffff8118291f>] vm_mmap_pgoff+0x6f/0xc0
> [ 403.117987]
> [ 403.117987] which lock already depends on the new lock.
> [ 403.117987]
> [ 403.117990]
> [ 403.117990] the existing dependency chain (in reverse order) is:
> [ 403.117992]
> [ 403.117992] -> #1 (&mm->mmap_sem){++++++}:
> [ 403.117997] [<ffffffff810d733c>] validate_chain.isra.39+0x5fc/0x9a0
> [ 403.118006] [<ffffffff810d8bc3>] __lock_acquire+0x4d3/0xd30
> [ 403.118010] [<ffffffff810d9da7>] lock_acquire+0xa7/0x160
> [ 403.118014] [<ffffffff8118c9ec>] might_fault+0x7c/0xb0
> [ 403.118018] [<ffffffffa0028a25>] video_usercopy+0x425/0x610 [videodev]
> [ 403.118028] [<ffffffffa0028c25>] video_ioctl2+0x15/0x20 [videodev]
> [ 403.118034] [<ffffffffa0022764>] v4l2_ioctl+0x184/0x1a0 [videodev]
> [ 403.118040] [<ffffffff811d77d0>] do_vfs_ioctl+0x2f0/0x4f0
> [ 403.118307] [<ffffffff811d7a51>] SyS_ioctl+0x81/0xa0
> [ 403.118311] [<ffffffff8199dc69>] system_call_fastpath+0x16/0x1b
> [ 403.118319]
> [ 403.118319] -> #0 (&dev->mutex#3){+.+.+.}:
> [ 403.118324] [<ffffffff810d6a96>] check_prevs_add+0x746/0x9f0
> [ 403.118329] [<ffffffff810d733c>] validate_chain.isra.39+0x5fc/0x9a0
> [ 403.118333] [<ffffffff810d8bc3>] __lock_acquire+0x4d3/0xd30
> [ 403.118336] [<ffffffff810d9da7>] lock_acquire+0xa7/0x160
> [ 403.118340] [<ffffffff81999664>] mutex_lock_interruptible_nested+0x64/0x640
> [ 403.118344] [<ffffffffa005a6c3>] vb2_fop_mmap+0x33/0x90 [videobuf2_core]
> [ 403.118349] [<ffffffffa0022122>] v4l2_mmap+0x62/0xa0 [videodev]
> [ 403.118354] [<ffffffff81197270>] mmap_region+0x3d0/0x5d0
> [ 403.118359] [<ffffffff8119778d>] do_mmap_pgoff+0x31d/0x400
> [ 403.118363] [<ffffffff81182940>] vm_mmap_pgoff+0x90/0xc0
> [ 403.118366] [<ffffffff81195cef>] SyS_mmap_pgoff+0x1df/0x2a0
> [ 403.118369] [<ffffffff810085c2>] SyS_mmap+0x22/0x30
> [ 403.118376] [<ffffffff8199dc69>] system_call_fastpath+0x16/0x1b
> [ 403.118381]
> [ 403.118381] other info that might help us debug this:
> [ 403.118381]
> [ 403.118383] Possible unsafe locking scenario:
> [ 403.118383]
> [ 403.118385] CPU0 CPU1
> [ 403.118387] ---- ----
> [ 403.118388] lock(&mm->mmap_sem);
> [ 403.118391] lock(&dev->mutex#3);
> [ 403.118394] lock(&mm->mmap_sem);
> [ 403.118397] lock(&dev->mutex#3);
> [ 403.118400]
> [ 403.118400] *** DEADLOCK ***
> [ 403.118400]
> [ 403.118403] 1 lock held by v4l2-ctl/15377:
> [ 403.118405] #0: (&mm->mmap_sem){++++++}, at: [<ffffffff8118291f>] vm_mmap_pgoff+0x6f/0xc0
> [ 403.118411]
> [ 403.118411] stack backtrace:
> [ 403.118415] CPU: 0 PID: 15377 Comm: v4l2-ctl Not tainted 3.16.0-rc6-test-media #961
> [ 403.118418] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/31/2013
> [ 403.118420] ffffffff82a6c9d0 ffff8800af37fb00 ffffffff819916a2 ffffffff82a6c9d0
> [ 403.118425] ffff8800af37fb40 ffffffff810d5715 ffff8802308e4200 0000000000000000
> [ 403.118429] ffff8802308e4a48 ffff8802308e4a48 ffff8802308e4200 0000000000000001
> [ 403.118433] Call Trace:
> [ 403.118441] [<ffffffff819916a2>] dump_stack+0x4e/0x7a
> [ 403.118445] [<ffffffff810d5715>] print_circular_bug+0x1d5/0x2a0
> [ 403.118449] [<ffffffff810d6a96>] check_prevs_add+0x746/0x9f0
> [ 403.118455] [<ffffffff8119c172>] ? find_vmap_area+0x42/0x70
> [ 403.118459] [<ffffffff810d733c>] validate_chain.isra.39+0x5fc/0x9a0
> [ 403.118463] [<ffffffff810d8bc3>] __lock_acquire+0x4d3/0xd30
> [ 403.118468] [<ffffffff810d9da7>] lock_acquire+0xa7/0x160
> [ 403.118472] [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
> [ 403.118476] [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
> [ 403.118480] [<ffffffff81999664>] mutex_lock_interruptible_nested+0x64/0x640
> [ 403.118484] [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
> [ 403.118488] [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
> [ 403.118493] [<ffffffff810d8055>] ? mark_held_locks+0x75/0xa0
> [ 403.118497] [<ffffffffa005a6c3>] vb2_fop_mmap+0x33/0x90 [videobuf2_core]
> [ 403.118502] [<ffffffffa0022122>] v4l2_mmap+0x62/0xa0 [videodev]
> [ 403.118506] [<ffffffff81197270>] mmap_region+0x3d0/0x5d0
> [ 403.118510] [<ffffffff8119778d>] do_mmap_pgoff+0x31d/0x400
> [ 403.118513] [<ffffffff81182940>] vm_mmap_pgoff+0x90/0xc0
> [ 403.118517] [<ffffffff81195cef>] SyS_mmap_pgoff+0x1df/0x2a0
> [ 403.118521] [<ffffffff810085c2>] SyS_mmap+0x22/0x30
> [ 403.118525] [<ffffffff8199dc69>] system_call_fastpath+0x16/0x1b
>
> The reason is that vb2_fop_mmap and vb2_fop_get_unmapped_area take the core lock
> while they are called with the mmap_sem semaphore held. But elsewhere in the code
> the core lock is taken first but calls to copy_to/from_user() can take the mmap_sem
> semaphore as well, potentially causing a classical A-B/B-A deadlock.
>
> However, the mmap/get_unmapped_area calls really shouldn't take the core lock
> at all. So what would happen if they don't take the core lock anymore?
>
> There are two situations that need to be taken into account: calling mmap while
> new buffers are being added and calling mmap while buffers are being deleted.
>
> The first case works almost fine without a lock: in all cases mmap relies on
> correctly filled-in q->num_buffers/q->num_planes values and those are only
> updated by reqbufs and create_buffers *after* any new buffers have been
> initialized completely. Except in one case: if an error occurred while allocating
> the buffers it will increase num_buffers and rely on __vb2_queue_free to
> decrease it again. So there is a short period where the buffer information
> may be wrong.
>
> The second case definitely does pose a problem: buffers may be in the process
> of being deleted, without the internal structure being updated.
>
> In order to fix this a new mutex is added to vb2_queue that is taken when
> buffers are allocated or deleted, and in vb2_mmap. That way vb2_mmap won't
> get stale buffer data. Note that this is a problem only for MEMORY_MMAP, so
> even though __qbuf_userptr and __qbuf_dmabuf also mess around with buffers
> (mem_priv in particular), this doesn't clash with vb2_mmap or
> vb2_get_unmapped_area since those are MMAP specific.
>
> As an additional bonus the hack in __buf_prepare, the USERPTR case, can be
> removed as well since mmap() no longer takes the core lock.
>
> All in all a much cleaner solution.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 56 +++++++++++---------------------
> include/media/videobuf2-core.h | 2 ++
> 2 files changed, 21 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index c359006..6554bf3 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -882,7 +882,9 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> * We already have buffers allocated, so first check if they
> * are not in use and can be freed.
> */
> + mutex_lock(&q->queue_lock);
> if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
> + mutex_unlock(&q->queue_lock);
> dprintk(1, "memory in use, cannot free\n");
> return -EBUSY;
> }
> @@ -894,6 +896,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> */
> __vb2_queue_cancel(q);
> ret = __vb2_queue_free(q, q->num_buffers);
> + mutex_unlock(&q->queue_lock);
> if (ret)
> return ret;
>
> @@ -955,6 +958,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> */
> }
>
> + mutex_lock(&q->queue_lock);
> q->num_buffers = allocated_buffers;
>
> if (ret < 0) {
> @@ -963,8 +967,10 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> * from q->num_buffers.
> */
> __vb2_queue_free(q, allocated_buffers);
> + mutex_unlock(&q->queue_lock);
> return ret;
> }
> + mutex_unlock(&q->queue_lock);
>
> /*
> * Return the number of successfully allocated buffers
> @@ -1061,6 +1067,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
> */
> }
>
> + mutex_lock(&q->queue_lock);
> q->num_buffers += allocated_buffers;
>
> if (ret < 0) {
> @@ -1069,8 +1076,10 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
> * from q->num_buffers.
> */
> __vb2_queue_free(q, allocated_buffers);
> + mutex_unlock(&q->queue_lock);
> return -ENOMEM;
> }
> + mutex_unlock(&q->queue_lock);
>
> /*
> * Return the number of successfully allocated buffers
> @@ -1582,7 +1591,6 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
> static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
> {
> struct vb2_queue *q = vb->vb2_queue;
> - struct rw_semaphore *mmap_sem;
> int ret;
>
> ret = __verify_length(vb, b);
> @@ -1619,26 +1627,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
> ret = __qbuf_mmap(vb, b);
> break;
> case V4L2_MEMORY_USERPTR:
> - /*
> - * In case of user pointer buffers vb2 allocators need to get
> - * direct access to userspace pages. This requires getting
> - * the mmap semaphore for read access in the current process
> - * structure. The same semaphore is taken before calling mmap
> - * operation, while both qbuf/prepare_buf and mmap are called
> - * by the driver or v4l2 core with the driver's lock held.
> - * To avoid an AB-BA deadlock (mmap_sem then driver's lock in
> - * mmap and driver's lock then mmap_sem in qbuf/prepare_buf),
> - * the videobuf2 core releases the driver's lock, takes
> - * mmap_sem and then takes the driver's lock again.
> - */
> - mmap_sem = ¤t->mm->mmap_sem;
> - call_void_qop(q, wait_prepare, q);
> - down_read(mmap_sem);
> - call_void_qop(q, wait_finish, q);
> -
> ret = __qbuf_userptr(vb, b);
> -
> - up_read(mmap_sem);
> break;
> case V4L2_MEMORY_DMABUF:
> ret = __qbuf_dmabuf(vb, b);
> @@ -2485,7 +2474,9 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
> return -EINVAL;
> }
>
> + mutex_lock(&q->queue_lock);
> ret = call_memop(vb, mmap, vb->planes[plane].mem_priv, vma);
> + mutex_unlock(&q->queue_lock);
> if (ret)
> return ret;
>
> @@ -2504,6 +2495,7 @@ unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
> unsigned long off = pgoff << PAGE_SHIFT;
> struct vb2_buffer *vb;
> unsigned int buffer, plane;
> + void *vaddr;
> int ret;
>
> if (q->memory != V4L2_MEMORY_MMAP) {
> @@ -2520,7 +2512,8 @@ unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
>
> vb = q->bufs[buffer];
>
> - return (unsigned long)vb2_plane_vaddr(vb, plane);
> + vaddr = vb2_plane_vaddr(vb, plane);
> + return vaddr ? (unsigned long)vaddr : -EINVAL;
> }
> EXPORT_SYMBOL_GPL(vb2_get_unmapped_area);
> #endif
> @@ -2660,6 +2653,7 @@ int vb2_queue_init(struct vb2_queue *q)
> INIT_LIST_HEAD(&q->queued_list);
> INIT_LIST_HEAD(&q->done_list);
> spin_lock_init(&q->done_lock);
> + mutex_init(&q->queue_lock);
> init_waitqueue_head(&q->done_wq);
>
> if (q->buf_struct_size == 0)
> @@ -2681,7 +2675,9 @@ void vb2_queue_release(struct vb2_queue *q)
> {
> __vb2_cleanup_fileio(q);
> __vb2_queue_cancel(q);
> + mutex_lock(&q->queue_lock);
> __vb2_queue_free(q, q->num_buffers);
> + mutex_unlock(&q->queue_lock);
> }
> EXPORT_SYMBOL_GPL(vb2_queue_release);
>
> @@ -3346,15 +3342,8 @@ EXPORT_SYMBOL_GPL(vb2_ioctl_expbuf);
> int vb2_fop_mmap(struct file *file, struct vm_area_struct *vma)
> {
> struct video_device *vdev = video_devdata(file);
> - struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock;
> - int err;
>
> - if (lock && mutex_lock_interruptible(lock))
> - return -ERESTARTSYS;
> - err = vb2_mmap(vdev->queue, vma);
> - if (lock)
> - mutex_unlock(lock);
> - return err;
> + return vb2_mmap(vdev->queue, vma);
> }
> EXPORT_SYMBOL_GPL(vb2_fop_mmap);
>
> @@ -3473,15 +3462,8 @@ unsigned long vb2_fop_get_unmapped_area(struct file *file, unsigned long addr,
> unsigned long len, unsigned long pgoff, unsigned long flags)
> {
> struct video_device *vdev = video_devdata(file);
> - struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock;
> - int ret;
>
> - if (lock && mutex_lock_interruptible(lock))
> - return -ERESTARTSYS;
> - ret = vb2_get_unmapped_area(vdev->queue, addr, len, pgoff, flags);
> - if (lock)
> - mutex_unlock(lock);
> - return ret;
> + return vb2_get_unmapped_area(vdev->queue, addr, len, pgoff, flags);
> }
> EXPORT_SYMBOL_GPL(vb2_fop_get_unmapped_area);
> #endif
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index fc910a6..ae1289b 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -366,6 +366,7 @@ struct v4l2_fh;
> * cannot be started unless at least this number of buffers
> * have been queued into the driver.
> *
> + * @queue_lock: private mutex used when buffers are allocated/freed/mmapped
> * @memory: current memory type used
> * @bufs: videobuf buffer structures
> * @num_buffers: number of allocated/used buffers
> @@ -399,6 +400,7 @@ struct vb2_queue {
> u32 min_buffers_needed;
>
> /* private: internal use only */
> + struct mutex queue_lock;
> enum v4l2_memory memory;
> struct vb2_buffer *bufs[VIDEO_MAX_FRAME];
> unsigned int num_buffers;
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCHv3] videobuf2: fix lockdep warning
2014-08-05 10:56 [PATCHv3] videobuf2: fix lockdep warning Hans Verkuil
2014-08-05 11:43 ` Marek Szyprowski
@ 2014-08-05 22:15 ` Laurent Pinchart
2014-08-06 6:44 ` Hans Verkuil
1 sibling, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2014-08-05 22:15 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Pawel Osciak, Marek Szyprowski, sasha.levin
Hi Hans,
Thank you for the patch.
On Tuesday 05 August 2014 12:56:14 Hans Verkuil wrote:
> Changes since v2: use a mutex instead of a spinlock due to possible sleeps
> inside the mmap memop. Use the mutex when buffers are allocated/freed, so
> it's a much coarser lock.
>
> The following lockdep warning has been there ever since commit
> a517cca6b24fc54ac209e44118ec8962051662e3 one year ago:
[snip]
> The reason is that vb2_fop_mmap and vb2_fop_get_unmapped_area take the core
> lock while they are called with the mmap_sem semaphore held. But elsewhere
> in the code the core lock is taken first but calls to copy_to/from_user()
> can take the mmap_sem semaphore as well, potentially causing a classical
> A-B/B-A deadlock.
>
> However, the mmap/get_unmapped_area calls really shouldn't take the core
> lock at all. So what would happen if they don't take the core lock anymore?
>
> There are two situations that need to be taken into account: calling mmap
> while new buffers are being added and calling mmap while buffers are being
> deleted.
>
> The first case works almost fine without a lock: in all cases mmap relies on
> correctly filled-in q->num_buffers/q->num_planes values and those are only
> updated by reqbufs and create_buffers *after* any new buffers have been
> initialized completely. Except in one case: if an error occurred while
> allocating the buffers it will increase num_buffers and rely on
> __vb2_queue_free to decrease it again. So there is a short period where the
> buffer information may be wrong.
>
> The second case definitely does pose a problem: buffers may be in the
> process of being deleted, without the internal structure being updated.
>
> In order to fix this a new mutex is added to vb2_queue that is taken when
> buffers are allocated or deleted, and in vb2_mmap. That way vb2_mmap won't
> get stale buffer data. Note that this is a problem only for MEMORY_MMAP, so
> even though __qbuf_userptr and __qbuf_dmabuf also mess around with buffers
> (mem_priv in particular), this doesn't clash with vb2_mmap or
> vb2_get_unmapped_area since those are MMAP specific.
>
> As an additional bonus the hack in __buf_prepare, the USERPTR case, can be
> removed as well since mmap() no longer takes the core lock.
>
> All in all a much cleaner solution.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
with one small comment, please see below.
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 56 ++++++++++-------------------
> include/media/videobuf2-core.h | 2 ++
> 2 files changed, 21 insertions(+), 37 deletions(-)
[snip]
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index fc910a6..ae1289b 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -366,6 +366,7 @@ struct v4l2_fh;
> * cannot be started unless at least this number of buffers
> * have been queued into the driver.
> *
> + * @queue_lock: private mutex used when buffers are allocated/freed/mmapped
queue_lock sounds a bit too generic to me, it might confuse readers into
thinking the lock protects the whole queue.
> * @memory: current memory type used
> * @bufs: videobuf buffer structures
> * @num_buffers: number of allocated/used buffers
> @@ -399,6 +400,7 @@ struct vb2_queue {
> u32 min_buffers_needed;
>
> /* private: internal use only */
> + struct mutex queue_lock;
> enum v4l2_memory memory;
> struct vb2_buffer *bufs[VIDEO_MAX_FRAME];
> unsigned int num_buffers;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCHv3] videobuf2: fix lockdep warning
2014-08-05 22:15 ` Laurent Pinchart
@ 2014-08-06 6:44 ` Hans Verkuil
2014-08-06 8:46 ` Laurent Pinchart
0 siblings, 1 reply; 5+ messages in thread
From: Hans Verkuil @ 2014-08-06 6:44 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, Pawel Osciak, Marek Szyprowski, sasha.levin
On 08/06/2014 12:15 AM, Laurent Pinchart wrote:
> Hi Hans,
>
> Thank you for the patch.
>
> On Tuesday 05 August 2014 12:56:14 Hans Verkuil wrote:
>> Changes since v2: use a mutex instead of a spinlock due to possible sleeps
>> inside the mmap memop. Use the mutex when buffers are allocated/freed, so
>> it's a much coarser lock.
>>
>> The following lockdep warning has been there ever since commit
>> a517cca6b24fc54ac209e44118ec8962051662e3 one year ago:
>
> [snip]
>
>> The reason is that vb2_fop_mmap and vb2_fop_get_unmapped_area take the core
>> lock while they are called with the mmap_sem semaphore held. But elsewhere
>> in the code the core lock is taken first but calls to copy_to/from_user()
>> can take the mmap_sem semaphore as well, potentially causing a classical
>> A-B/B-A deadlock.
>>
>> However, the mmap/get_unmapped_area calls really shouldn't take the core
>> lock at all. So what would happen if they don't take the core lock anymore?
>>
>> There are two situations that need to be taken into account: calling mmap
>> while new buffers are being added and calling mmap while buffers are being
>> deleted.
>>
>> The first case works almost fine without a lock: in all cases mmap relies on
>> correctly filled-in q->num_buffers/q->num_planes values and those are only
>> updated by reqbufs and create_buffers *after* any new buffers have been
>> initialized completely. Except in one case: if an error occurred while
>> allocating the buffers it will increase num_buffers and rely on
>> __vb2_queue_free to decrease it again. So there is a short period where the
>> buffer information may be wrong.
>>
>> The second case definitely does pose a problem: buffers may be in the
>> process of being deleted, without the internal structure being updated.
>>
>> In order to fix this a new mutex is added to vb2_queue that is taken when
>> buffers are allocated or deleted, and in vb2_mmap. That way vb2_mmap won't
>> get stale buffer data. Note that this is a problem only for MEMORY_MMAP, so
>> even though __qbuf_userptr and __qbuf_dmabuf also mess around with buffers
>> (mem_priv in particular), this doesn't clash with vb2_mmap or
>> vb2_get_unmapped_area since those are MMAP specific.
>>
>> As an additional bonus the hack in __buf_prepare, the USERPTR case, can be
>> removed as well since mmap() no longer takes the core lock.
>>
>> All in all a much cleaner solution.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> with one small comment, please see below.
>
>> ---
>> drivers/media/v4l2-core/videobuf2-core.c | 56 ++++++++++-------------------
>> include/media/videobuf2-core.h | 2 ++
>> 2 files changed, 21 insertions(+), 37 deletions(-)
>
> [snip]
>
>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>> index fc910a6..ae1289b 100644
>> --- a/include/media/videobuf2-core.h
>> +++ b/include/media/videobuf2-core.h
>> @@ -366,6 +366,7 @@ struct v4l2_fh;
>> * cannot be started unless at least this number of buffers
>> * have been queued into the driver.
>> *
>> + * @queue_lock: private mutex used when buffers are allocated/freed/mmapped
>
> queue_lock sounds a bit too generic to me, it might confuse readers into
> thinking the lock protects the whole queue.
How about mmap_lock?
Regards,
Hans
>
>> * @memory: current memory type used
>> * @bufs: videobuf buffer structures
>> * @num_buffers: number of allocated/used buffers
>> @@ -399,6 +400,7 @@ struct vb2_queue {
>> u32 min_buffers_needed;
>>
>> /* private: internal use only */
>> + struct mutex queue_lock;
>> enum v4l2_memory memory;
>> struct vb2_buffer *bufs[VIDEO_MAX_FRAME];
>> unsigned int num_buffers;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCHv3] videobuf2: fix lockdep warning
2014-08-06 6:44 ` Hans Verkuil
@ 2014-08-06 8:46 ` Laurent Pinchart
0 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2014-08-06 8:46 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Pawel Osciak, Marek Szyprowski, sasha.levin
Hi Hans,
On Wednesday 06 August 2014 08:44:27 Hans Verkuil wrote:
> On 08/06/2014 12:15 AM, Laurent Pinchart wrote:
> > On Tuesday 05 August 2014 12:56:14 Hans Verkuil wrote:
> >> Changes since v2: use a mutex instead of a spinlock due to possible
> >> sleeps inside the mmap memop. Use the mutex when buffers are
> >> allocated/freed, so it's a much coarser lock.
> >>
> >> The following lockdep warning has been there ever since commit
> >
> >> a517cca6b24fc54ac209e44118ec8962051662e3 one year ago:
> > [snip]
> >
> >> The reason is that vb2_fop_mmap and vb2_fop_get_unmapped_area take the
> >> core lock while they are called with the mmap_sem semaphore held. But
> >> elsewhere in the code the core lock is taken first but calls to
> >> copy_to/from_user() can take the mmap_sem semaphore as well, potentially
> >> causing a classical A-B/B-A deadlock.
> >>
> >> However, the mmap/get_unmapped_area calls really shouldn't take the core
> >> lock at all. So what would happen if they don't take the core lock
> >> anymore?
> >>
> >> There are two situations that need to be taken into account: calling mmap
> >> while new buffers are being added and calling mmap while buffers are
> >> being deleted.
> >>
> >> The first case works almost fine without a lock: in all cases mmap relies
> >> on correctly filled-in q->num_buffers/q->num_planes values and those are
> >> only updated by reqbufs and create_buffers *after* any new buffers have
> >> been initialized completely. Except in one case: if an error occurred
> >> while allocating the buffers it will increase num_buffers and rely on
> >> __vb2_queue_free to decrease it again. So there is a short period where
> >> the buffer information may be wrong.
> >>
> >> The second case definitely does pose a problem: buffers may be in the
> >> process of being deleted, without the internal structure being updated.
> >>
> >> In order to fix this a new mutex is added to vb2_queue that is taken when
> >> buffers are allocated or deleted, and in vb2_mmap. That way vb2_mmap
> >> won't get stale buffer data. Note that this is a problem only for
> >> MEMORY_MMAP, so even though __qbuf_userptr and __qbuf_dmabuf also mess
> >> around with buffers (mem_priv in particular), this doesn't clash with
> >> vb2_mmap or vb2_get_unmapped_area since those are MMAP specific.
> >>
> >> As an additional bonus the hack in __buf_prepare, the USERPTR case, can
> >> be removed as well since mmap() no longer takes the core lock.
> >>
> >> All in all a much cleaner solution.
> >>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > with one small comment, please see below.
> >
> >> ---
> >>
> >> drivers/media/v4l2-core/videobuf2-core.c | 56 +++++++++-----------------
> >> include/media/videobuf2-core.h | 2 ++
> >> 2 files changed, 21 insertions(+), 37 deletions(-)
> >
> > [snip]
> >
> >> diff --git a/include/media/videobuf2-core.h
> >> b/include/media/videobuf2-core.h index fc910a6..ae1289b 100644
> >> --- a/include/media/videobuf2-core.h
> >> +++ b/include/media/videobuf2-core.h
> >> @@ -366,6 +366,7 @@ struct v4l2_fh;
> >> * cannot be started unless at least this number of buffers
> >> * have been queued into the driver.
> >> *
> >> + * @queue_lock: private mutex used when buffers are
> >> allocated/freed/mmapped
> >
> > queue_lock sounds a bit too generic to me, it might confuse readers into
> > thinking the lock protects the whole queue.
>
> How about mmap_lock?
That sounds better.
> >> * @memory: current memory type used
> >> * @bufs: videobuf buffer structures
> >> * @num_buffers: number of allocated/used buffers
> >> @@ -399,6 +400,7 @@ struct vb2_queue {
> >> u32 min_buffers_needed;
> >>
> >> /* private: internal use only */
> >> + struct mutex queue_lock;
> >> enum v4l2_memory memory;
> >> struct vb2_buffer *bufs[VIDEO_MAX_FRAME];
> >> unsigned int num_buffers;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-08-06 8:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-05 10:56 [PATCHv3] videobuf2: fix lockdep warning Hans Verkuil
2014-08-05 11:43 ` Marek Szyprowski
2014-08-05 22:15 ` Laurent Pinchart
2014-08-06 6:44 ` Hans Verkuil
2014-08-06 8:46 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox