diff mbox

[v2] media: s5p-mfc: fix lockdep warning

Message ID 20171030210457.17583-1-shuahkh@osg.samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shuah Khan Oct. 30, 2017, 9:04 p.m. UTC
The driver mmap functions shouldn't take lock when calling vb2_mmap().
Fix it to not take the lock. The following lockdep warning is fixed
with this change.

[ 2106.181412] ======================================================
[ 2106.187563] WARNING: possible circular locking dependency detected
[ 2106.193718] 4.14.0-rc2-00002-gfab205f-dirty #4 Not tainted
[ 2106.199175] ------------------------------------------------------
[ 2106.205328] qtdemux0:sink/2614 is trying to acquire lock:
[ 2106.210701]  (&dev->mfc_mutex){+.+.}, at: [<bf175544>] s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
[ 2106.218672]
[ 2106.218672] but task is already holding lock:
[ 2106.224477]  (&mm->mmap_sem){++++}, at: [<c01df2e4>] vm_mmap_pgoff+0x44/0xb8
[ 2106.231497]
[ 2106.231497] which lock already depends on the new lock.
[ 2106.231497]
[ 2106.239642]
[ 2106.239642] the existing dependency chain (in reverse order) is:
[ 2106.247095]
[ 2106.247095] -> #1 (&mm->mmap_sem){++++}:
[ 2106.252473]        __might_fault+0x80/0xb0
[ 2106.256567]        video_usercopy+0x1cc/0x510 [videodev]
[ 2106.261845]        v4l2_ioctl+0xa4/0xdc [videodev]
[ 2106.266596]        do_vfs_ioctl+0xa0/0xa18
[ 2106.270667]        SyS_ioctl+0x34/0x5c
[ 2106.274395]        ret_fast_syscall+0x0/0x28
[ 2106.278637]
[ 2106.278637] -> #0 (&dev->mfc_mutex){+.+.}:
[ 2106.284186]        lock_acquire+0x6c/0x88
[ 2106.288173]        __mutex_lock+0x68/0xa34
[ 2106.292244]        mutex_lock_interruptible_nested+0x1c/0x24
[ 2106.297893]        s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
[ 2106.302747]        v4l2_mmap+0x54/0x88 [videodev]
[ 2106.307409]        mmap_region+0x3a8/0x638
[ 2106.311480]        do_mmap+0x330/0x3a4
[ 2106.315207]        vm_mmap_pgoff+0x90/0xb8
[ 2106.319279]        SyS_mmap_pgoff+0x90/0xc0
[ 2106.323439]        ret_fast_syscall+0x0/0x28
[ 2106.327683]
[ 2106.327683] other info that might help us debug this:
[ 2106.327683]
[ 2106.335656]  Possible unsafe locking scenario:
[ 2106.335656]
[ 2106.341548]        CPU0                    CPU1
[ 2106.346053]        ----                    ----
[ 2106.350559]   lock(&mm->mmap_sem);
[ 2106.353939]                                lock(&dev->mfc_mutex);
[ 2106.353939]                                lock(&dev->mfc_mutex);
[ 2106.365897]   lock(&dev->mfc_mutex);
[ 2106.369450]
[ 2106.369450]  *** DEADLOCK ***
[ 2106.369450]
[ 2106.375344] 1 lock held by qtdemux0:sink/2614:
[ 2106.379762]  #0:  (&mm->mmap_sem){++++}, at: [<c01df2e4>] vm_mmap_pgoff+0x44/0xb8
[ 2106.387214]
[ 2106.387214] stack backtrace:
[ 2106.391550] CPU: 7 PID: 2614 Comm: qtdemux0:sink Not tainted 4.14.0-rc2-00002-gfab205f-dirty #4
[ 2106.400213] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 2106.406285] [<c01102c8>] (unwind_backtrace) from [<c010cabc>] (show_stack+0x10/0x14)
[ 2106.413995] [<c010cabc>] (show_stack) from [<c08543a4>] (dump_stack+0x98/0xc4)
[ 2106.421187] [<c08543a4>] (dump_stack) from [<c016b2fc>] (print_circular_bug+0x254/0x410)
[ 2106.429245] [<c016b2fc>] (print_circular_bug) from [<c016c580>] (check_prev_add+0x468/0x938)
[ 2106.437651] [<c016c580>] (check_prev_add) from [<c016f4dc>] (__lock_acquire+0x1314/0x14fc)
[ 2106.445883] [<c016f4dc>] (__lock_acquire) from [<c016fefc>] (lock_acquire+0x6c/0x88)
[ 2106.453596] [<c016fefc>] (lock_acquire) from [<c0869fb4>] (__mutex_lock+0x68/0xa34)
[ 2106.461221] [<c0869fb4>] (__mutex_lock) from [<c086aa08>] (mutex_lock_interruptible_nested+0x1c/0x24)
[ 2106.470425] [<c086aa08>] (mutex_lock_interruptible_nested) from [<bf175544>] (s5p_mfc_mmap+0x28/0xd4 [s5p_mfc])
[ 2106.480494] [<bf175544>] (s5p_mfc_mmap [s5p_mfc]) from [<bf037120>] (v4l2_mmap+0x54/0x88 [videodev])
[ 2106.489575] [<bf037120>] (v4l2_mmap [videodev]) from [<c01f4798>] (mmap_region+0x3a8/0x638)
[ 2106.497875] [<c01f4798>] (mmap_region) from [<c01f4d58>] (do_mmap+0x330/0x3a4)
[ 2106.505068] [<c01f4d58>] (do_mmap) from [<c01df330>] (vm_mmap_pgoff+0x90/0xb8)
[ 2106.512260] [<c01df330>] (vm_mmap_pgoff) from [<c01f28cc>] (SyS_mmap_pgoff+0x90/0xc0)
[ 2106.520059] [<c01f28cc>] (SyS_mmap_pgoff) from [<c0108820>] (ret_fast_syscall+0x0/0x28)

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
Suggested-by: Hans Verkuil <hansverk@cisco.com>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Hans Verkuil <hansverk@cisco.com>
---

Changes since v1:
- Fixed unused warning. Added Han's Ack

 drivers/media/platform/s5p-mfc/s5p_mfc.c | 4 ----
 1 file changed, 4 deletions(-)
diff mbox

Patch

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 4c253fb..3611016 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1043,12 +1043,9 @@  static unsigned int s5p_mfc_poll(struct file *file,
 static int s5p_mfc_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct s5p_mfc_ctx *ctx = fh_to_ctx(file->private_data);
-	struct s5p_mfc_dev *dev = ctx->dev;
 	unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
 	int ret;
 
-	if (mutex_lock_interruptible(&dev->mfc_mutex))
-		return -ERESTARTSYS;
 	if (offset < DST_QUEUE_OFF_BASE) {
 		mfc_debug(2, "mmaping source\n");
 		ret = vb2_mmap(&ctx->vq_src, vma);
@@ -1057,7 +1054,6 @@  static int s5p_mfc_mmap(struct file *file, struct vm_area_struct *vma)
 		vma->vm_pgoff -= (DST_QUEUE_OFF_BASE >> PAGE_SHIFT);
 		ret = vb2_mmap(&ctx->vq_dst, vma);
 	}
-	mutex_unlock(&dev->mfc_mutex);
 	return ret;
 }