From c13a5ccf5da86239213033214658b8a170eeab87 Mon Sep 17 00:00:00 2001 From: Philipp Zabel Date: Fri, 10 Jul 2015 10:49:25 -0300 Subject: [media] v4l2-mem2mem: set the queue owner field just as vb2_ioctl_reqbufs does The queue owner will be used by videobuf2 trace events to determine and record the device minor number. It is set in v4l2_m2m_reqbufs instead of v4l2_m2m_ioctl_reqbufs because several drivers implement their own vidioc_reqbufs handlers that still call v4l2_m2m_reqbufs directly. Signed-off-by: Philipp Zabel Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-mem2mem.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers/media/v4l2-core/v4l2-mem2mem.c') diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c index dc853e57f91f..af8d6b4aa807 100644 --- a/drivers/media/v4l2-core/v4l2-mem2mem.c +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c @@ -357,9 +357,16 @@ int v4l2_m2m_reqbufs(struct file *file, struct v4l2_m2m_ctx *m2m_ctx, struct v4l2_requestbuffers *reqbufs) { struct vb2_queue *vq; + int ret; vq = v4l2_m2m_get_vq(m2m_ctx, reqbufs->type); - return vb2_reqbufs(vq, reqbufs); + ret = vb2_reqbufs(vq, reqbufs); + /* If count == 0, then the owner has released all buffers and he + is no longer owner of the queue. Otherwise we have an owner. */ + if (ret == 0) + vq->owner = reqbufs->count ? file->private_data : NULL; + + return ret; } EXPORT_SYMBOL_GPL(v4l2_m2m_reqbufs); -- cgit v1.2.3 From e752577ed7bf55c81e10343fced8b378cda2b63b Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Mon, 20 Jul 2015 04:58:24 -0300 Subject: [media] v4l2-mem2mem: drop lock in v4l2_m2m_fop_mmap The v4l2_m2m_fop_mmap function takes the core mutex, but this will result in a potential circular locking dependency: [ 262.517164] ====================================================== [ 262.517166] [ INFO: possible circular locking dependency detected ] [ 262.517169] 4.2.0-rc2-koryphon #844 Not tainted [ 262.517171] ------------------------------------------------------- [ 262.517173] v4l2-compliance/1379 is trying to acquire lock: [ 262.517175] (&dev->dev_mutex){+.+.+.}, at: [] v4l2_m2m_fop_mmap+0x2b/0x90 [v4l2_mem2mem] [ 262.517187] but task is already holding lock: [ 262.517189] (&mm->mmap_sem){++++++}, at: [] vm_mmap_pgoff+0x69/0xc0 [ 262.517199] which lock already depends on the new lock. [ 262.517202] the existing dependency chain (in reverse order) is: [ 262.517204] -> #1 (&mm->mmap_sem){++++++}: [ 262.517209] [] __lock_acquire+0x62b/0xe80 [ 262.517215] [] lock_acquire+0x65/0x90 [ 262.517218] [] __might_fault+0x75/0xa0 [ 262.517222] [] video_usercopy+0x3e9/0x4e0 [videodev] [ 262.517231] [] video_ioctl2+0x10/0x20 [videodev] [ 262.517238] [] v4l2_ioctl+0xc3/0xe0 [videodev] [ 262.517243] [] do_vfs_ioctl+0x2fc/0x550 [ 262.517248] [] SyS_ioctl+0x74/0x80 [ 262.517252] [] entry_SYSCALL_64_fastpath+0x12/0x76 [ 262.517258] -> #0 (&dev->dev_mutex){+.+.+.}: [ 262.517262] [] validate_chain.isra.38+0xd04/0x1170 [ 262.517266] [] __lock_acquire+0x62b/0xe80 [ 262.517270] [] lock_acquire+0x65/0x90 [ 262.517273] [] mutex_lock_interruptible_nested+0x6c/0x4b0 [ 262.517279] [] v4l2_m2m_fop_mmap+0x2b/0x90 [v4l2_mem2mem] [ 262.517284] [] v4l2_mmap+0x4f/0x90 [videodev] [ 262.517288] [] mmap_region+0x38c/0x5b0 [ 262.517293] [] do_mmap_pgoff+0x2f5/0x3e0 [ 262.517297] [] vm_mmap_pgoff+0x8a/0xc0 [ 262.517300] [] SyS_mmap_pgoff+0x1cb/0x270 [ 262.517304] [] SyS_mmap+0x1d/0x20 [ 262.517309] [] entry_SYSCALL_64_fastpath+0x12/0x76 [ 262.517313] other info that might help us debug this: [ 262.517315] Possible unsafe locking scenario: [ 262.517318] CPU0 CPU1 [ 262.517319] ---- ---- [ 262.517321] lock(&mm->mmap_sem); [ 262.517324] lock(&dev->dev_mutex); [ 262.517327] lock(&mm->mmap_sem); [ 262.517329] lock(&dev->dev_mutex); [ 262.517332] *** DEADLOCK *** Since vb2_fop_mmap doesn't take the lock, neither should v4l2_m2m_fop_mmap. Signed-off-by: Hans Verkuil Tested-by: Mikhail Ulyanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-mem2mem.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) (limited to 'drivers/media/v4l2-core/v4l2-mem2mem.c') diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c index af8d6b4aa807..ec3ad4eb0c57 100644 --- a/drivers/media/v4l2-core/v4l2-mem2mem.c +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c @@ -881,18 +881,8 @@ EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_streamoff); int v4l2_m2m_fop_mmap(struct file *file, struct vm_area_struct *vma) { struct v4l2_fh *fh = file->private_data; - struct v4l2_m2m_ctx *m2m_ctx = fh->m2m_ctx; - int ret; - - if (m2m_ctx->q_lock && mutex_lock_interruptible(m2m_ctx->q_lock)) - return -ERESTARTSYS; - - ret = v4l2_m2m_mmap(file, m2m_ctx, vma); - if (m2m_ctx->q_lock) - mutex_unlock(m2m_ctx->q_lock); - - return ret; + return v4l2_m2m_mmap(file, fh->m2m_ctx, vma); } EXPORT_SYMBOL_GPL(v4l2_m2m_fop_mmap); -- cgit v1.2.3