Skip to content

Commit

Permalink
MLK-24389 media: mx6s_capture: use vb2_fop_mmap instead
Browse files Browse the repository at this point in the history
use existing standard function instead of mx6s_csi_mmap().
this could also fix possible deadlock issue from mx6s_csi_mmap as below.

 ======================================================
 WARNING: possible circular locking dependency detected
 5.7.0-next-20200612-lts-next+g2a193301c3f1 #1 Tainted: G           O
 ------------------------------------------------------
 v4l2_capture_em/1430 is trying to acquire lock:
 d933281c (&mm->mmap_lock){++++}-{3:3}, at: get_vaddr_frames+0x5c/0x234

 but task is already holding lock:
 d94d6b50 (&csi_dev->lock){+.+.}-{3:3}, at: __video_do_ioctl+0xec/0x44c

 which lock already depends on the new lock.

 the existing dependency chain (in reverse order) is:

 -> #1 (&csi_dev->lock){+.+.}-{3:3}:
        mutex_lock_interruptible_nested+0x1c/0x24
        mx6s_csi_mmap+0x20/0x54 [mx6s_capture]
        v4l2_mmap+0x54/0x90
        mmap_region+0x3ac/0x67c
        do_mmap+0x3c0/0x50c
        vm_mmap_pgoff+0x94/0xe8
        ksys_mmap_pgoff+0x7c/0xb4
        ret_fast_syscall+0x0/0x28
        0xbe925524

 -> #0 (&mm->mmap_lock){++++}-{3:3}:
        lock_acquire+0xe0/0x524
        down_read+0x38/0x1f4
        get_vaddr_frames+0x5c/0x234
        vb2_create_framevec+0x48/0x84
        vb2_dc_get_userptr+0x7c/0x3e0
        __prepare_userptr+0x17c/0x3d0
        __buf_prepare+0x190/0x230
        vb2_core_qbuf+0x3c8/0x68c
        vb2_qbuf+0x7c/0xd4
        __video_do_ioctl+0x214/0x44c
        video_usercopy+0x140/0x860
        ksys_ioctl+0xec/0xb8c
        ret_fast_syscall+0x0/0x28
        0xbeb775a4

 other info that might help us debug this:

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(&csi_dev->lock);
                                lock(&mm->mmap_lock);
                                lock(&csi_dev->lock);
   lock(&mm->mmap_lock);

  *** DEADLOCK ***

 1 lock held by v4l2_capture_em/1430:
  #0: d94d6b50 (&csi_dev->lock){+.+.}-{3:3}, at: __video_do_ioctl+0xec/0x44c

 stack backtrace:
 CPU: 0 PID: 1430 Comm: v4l2_capture_em Tainted: G           O      5.7.0-next-20200612-lts-next+g2a193301c3f1 #1
 Hardware name: Freescale i.MX6 SoloX (Device Tree)
 [<c01127b0>] (unwind_backtrace) from [<c010c3b4>] (show_stack+0x10/0x14)
 [<c010c3b4>] (show_stack) from [<c05b8054>] (dump_stack+0xd8/0x10c)
 [<c05b8054>] (dump_stack) from [<c0193608>] (check_noncircular+0x130/0x1e4)
 [<c0193608>] (check_noncircular) from [<c0196580>] (__lock_acquire+0x15e8/0x33dc)
 [<c0196580>] (__lock_acquire) from [<c0198d0c>] (lock_acquire+0xe0/0x524)
 [<c0198d0c>] (lock_acquire) from [<c0e3f140>] (down_read+0x38/0x1f4)
 [<c0e3f140>] (down_read) from [<c02ba824>] (get_vaddr_frames+0x5c/0x234)
 [<c02ba824>] (get_vaddr_frames) from [<c09bbc48>] (vb2_create_framevec+0x48/0x84)
 [<c09bbc48>] (vb2_create_framevec) from [<c09bcf0c>] (vb2_dc_get_userptr+0x7c/0x3e0)
 [<c09bcf0c>] (vb2_dc_get_userptr) from [<c09b5368>] (__prepare_userptr+0x17c/0x3d0)
 [<c09b5368>] (__prepare_userptr) from [<c09b5e70>] (__buf_prepare+0x190/0x230)
 [<c09b5e70>] (__buf_prepare) from [<c09b80d4>] (vb2_core_qbuf+0x3c8/0x68c)
 [<c09b80d4>] (vb2_core_qbuf) from [<c09bb8bc>] (vb2_qbuf+0x7c/0xd4)
 [<c09bb8bc>] (vb2_qbuf) from [<c0988d8c>] (__video_do_ioctl+0x214/0x44c)
 [<c0988d8c>] (__video_do_ioctl) from [<c0989778>] (video_usercopy+0x140/0x860)
 [<c0989778>] (video_usercopy) from [<c02d6210>] (ksys_ioctl+0xec/0xb8c)
 [<c02d6210>] (ksys_ioctl) from [<c0100080>] (ret_fast_syscall+0x0/0x28)

Signed-off-by: Robby Cai <[email protected]>
Reviewed-by: G.n. Zhou <[email protected]>
  • Loading branch information
Robby Cai committed Jul 10, 2020
1 parent 49af6b3 commit c374959
Showing 1 changed file with 1 addition and 19 deletions.
20 changes: 1 addition & 19 deletions drivers/media/platform/mxc/capture/mx6s_capture.c
Original file line number Diff line number Diff line change
Expand Up @@ -1252,32 +1252,14 @@ static ssize_t mx6s_csi_read(struct file *file, char __user *buf,
return ret;
}

static int mx6s_csi_mmap(struct file *file, struct vm_area_struct *vma)
{
struct mx6s_csi_dev *csi_dev = video_drvdata(file);
int ret;

if (mutex_lock_interruptible(&csi_dev->lock))
return -ERESTARTSYS;
ret = vb2_mmap(&csi_dev->vb2_vidq, vma);
mutex_unlock(&csi_dev->lock);

pr_debug("vma start=0x%08lx, size=%ld, ret=%d\n",
(unsigned long)vma->vm_start,
(unsigned long)vma->vm_end-(unsigned long)vma->vm_start,
ret);

return ret;
}

static struct v4l2_file_operations mx6s_csi_fops = {
.owner = THIS_MODULE,
.open = mx6s_csi_open,
.release = mx6s_csi_close,
.read = mx6s_csi_read,
.poll = vb2_fop_poll,
.unlocked_ioctl = video_ioctl2, /* V4L2 ioctl handler */
.mmap = mx6s_csi_mmap,
.mmap = vb2_fop_mmap,
};

/*
Expand Down

0 comments on commit c374959

Please sign in to comment.