Hi Sanser,
On Sunday 06 May 2012 16:54:40 Sander Eikelenboom wrote:
> Hello Laurent / Mauro,
>
> I have updated to latest 3.4-rc5-tip, running multiple video grabbers.
> I don't see anything specific to uvcvideo anymore, but i do get the possible
> circular locking dependency below.
Thanks for the report.
We indeed have a serious issue there (CC'ing Hans Verkuil).
Hans, serializing both ioctl handling an mmap with a single device lock as we
currently do in V4L2 is prone to AB-BA deadlocks (uvcvideo shouldn't be
affected as it has no device-wide lock).
If we want to keep a device-wide lock we need to take it after the mm-
>mmap_sem lock in all code paths, as there's no way we can change the lock
ordering for mmap(). The copy_from_user/copy_to_user issue could be solved by
moving locking from v4l2_ioctl to __video_do_ioctl (device-wide locks would
then require using video_ioctl2), but I'm not sure whether that will play
nicely with the QBUF implementation in videobuf2 (which already includes a
workaround for this particular AB-BA deadlock issue).
> [ 96.257129] ======================================================
> [ 96.257129] [ INFO: possible circular locking dependency detected ]
> [ 96.257129] 3.4.0-rc5-20120506+ #4 Not tainted
> [ 96.257129] -------------------------------------------------------
> [ 96.257129] motion/2289 is trying to acquire lock:
> [ 96.257129] (&dev->lock){+.+.+.}, at: [<ffffffff8156c04c>]
> v4l2_mmap+0xbc/0xd0
> [ 96.257129]
> [ 96.257129] but task is already holding lock:
> [ 96.257129] (&mm->mmap_sem){++++++}, at: [<ffffffff81113cca>]
> sys_mmap_pgoff+0x1ca/0x220
> [ 96.257129]
> [ 96.257129] which lock already depends on the new lock.
> [ 96.257129]
> [ 96.257129]
> [ 96.257129] the existing dependency chain (in reverse order) is:
> [ 96.257129]
> [ 96.257129] -> #1 (&mm->mmap_sem){++++++}:
> [ 96.257129] [<ffffffff810aa972>] __lock_acquire+0x3a2/0xc50
> [ 96.257129] [<ffffffff810ab2e8>] lock_acquire+0xc8/0x110
> [ 96.257129] [<ffffffff81108cb8>] might_fault+0x68/0x90
> [ 96.257129] [<ffffffff8156d2a1>] video_usercopy+0x1d1/0x4e0
> [ 96.257129] [<ffffffff8156d5c0>] video_ioctl2+0x10/0x20
> [ 96.257129] [<ffffffff8156c110>] v4l2_ioctl+0xb0/0x180
> [ 96.257129] [<ffffffff8114dc3c>] do_vfs_ioctl+0x9c/0x580
> [ 96.257129] [<ffffffff8114e16a>] sys_ioctl+0x4a/0x80
> [ 96.257129] [<ffffffff81813039>] system_call_fastpath+0x16/0x1b
> [ 96.257129]
> [ 96.257129] -> #0 (&dev->lock){+.+.+.}:
> [ 96.257129] [<ffffffff810aa529>] validate_chain+0x1259/0x1300
> [ 96.257129] [<ffffffff810aa972>] __lock_acquire+0x3a2/0xc50
> [ 96.257129] [<ffffffff810ab2e8>] lock_acquire+0xc8/0x110
> [ 96.257129] [<ffffffff8180e58c>]
> mutex_lock_interruptible_nested+0x5c/0x550
> [ 96.257129] [<ffffffff8156c04c>] v4l2_mmap+0xbc/0xd0
> [ 96.257129] [<ffffffff8111359a>] mmap_region+0x3da/0x5a0
> [ 96.257129] [<ffffffff81113ab4>] do_mmap_pgoff+0x354/0x3a0
> [ 96.257129] [<ffffffff81113ce9>] sys_mmap_pgoff+0x1e9/0x220
> [ 96.257129] [<ffffffff81011ee9>] sys_mmap+0x29/0x30
> [ 96.257129] [<ffffffff81813039>] system_call_fastpath+0x16/0x1b
> [ 96.257129]
> [ 96.257129] other info that might help us debug this:
> [ 96.257129]
> [ 96.257129] Possible unsafe locking scenario:
> [ 96.257129]
> [ 96.257129] CPU0 CPU1
> [ 96.257129] ---- ----
> [ 96.257129] lock(&mm->mmap_sem);
> [ 96.257129] lock(&dev->lock);
> [ 96.257129] lock(&mm->mmap_sem);
> [ 96.257129] lock(&dev->lock);
> [ 96.257129]
> [ 96.257129] *** DEADLOCK ***
> [ 96.257129]
> [ 96.257129] 1 lock held by motion/2289:
> [ 96.257129] #0: (&mm->mmap_sem){++++++}, at: [<ffffffff81113cca>]
> sys_mmap_pgoff+0x1ca/0x220
> [ 96.257129]
> [ 96.257129] stack backtrace:
> [ 96.257129] Pid: 2289, comm: motion Not tainted 3.4.0-rc5-20120506+ #4
> [ 96.257129] Call Trace:
> [ 96.257129] [<ffffffff810a8fb6>] print_circular_bug+0x206/0x300
> [ 96.257129] [<ffffffff810aa529>] validate_chain+0x1259/0x1300
> [ 96.257129] [<ffffffff810a9417>] ? validate_chain+0x147/0x1300
> [ 96.257129] [<ffffffff810ab773>] ? lock_release+0x113/0x260
> [ 96.257129] [<ffffffff810aa972>] __lock_acquire+0x3a2/0xc50
> [ 96.257129] [<ffffffff810aa993>] ? __lock_acquire+0x3c3/0xc50
> [ 96.257129] [<ffffffff8156024c>] ? tda18271_tune+0x71c/0x9c0
> [ 96.257129] [<ffffffff810ab2e8>] lock_acquire+0xc8/0x110
> [ 96.257129] [<ffffffff8156c04c>] ? v4l2_mmap+0xbc/0xd0
> [ 96.257129] [<ffffffff8180e58c>]
> mutex_lock_interruptible_nested+0x5c/0x550
> [ 96.257129] [<ffffffff8156c04c>] ? v4l2_mmap+0xbc/0xd0
> [ 96.257129] [<ffffffff810a77ce>] ? mark_held_locks+0x9e/0x130
> [ 96.257129] [<ffffffff8156c04c>] ? v4l2_mmap+0xbc/0xd0
> [ 96.257129] [<ffffffff811134de>] ? mmap_region+0x31e/0x5a0
> [ 96.257129] [<ffffffff810a7900>] ? lockdep_trace_alloc+0xa0/0x130
> [ 96.257129] [<ffffffff8156c04c>] v4l2_mmap+0xbc/0xd0
> [ 96.257129] [<ffffffff8111359a>] mmap_region+0x3da/0x5a0
> [ 96.257129] [<ffffffff81113ab4>] do_mmap_pgoff+0x354/0x3a0
> [ 96.257129] [<ffffffff81113ce9>] sys_mmap_pgoff+0x1e9/0x220
> [ 96.257129] [<ffffffff8132554e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [ 96.257129] [<ffffffff81011ee9>] sys_mmap+0x29/0x30
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
