Hi Jan,
Thank you for the patch.
On Wednesday 02 October 2013 16:27:46 Jan Kara wrote:
> CC: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> CC: linux-media@xxxxxxxxxxxxxxx
> Signed-off-by: Jan Kara <jack@xxxxxxx>
Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Could you briefly explain where you're headed with this ? The V4L2 subsystem
has suffered for quite a long time from potentiel AB-BA deadlocks, due the
drivers taking the mmap_sem semaphore while holding the V4L2 buffers queue
lock in the code path below, and taking them in reverse order in the mmap()
path (as the mm core takes the mmap_sem semaphore before calling the mmap()
operation). We've solved that by releasing the V4L2 buffers queue lock before
taking the mmap_sem lock below and taking it back after releasing the mmap_sem
lock. Having an explicit indication that the mmap_sem lock was taken helped
keeping the problem in mind. I'm not against hiding it in
get_user_pages_fast(), but I'd like to know what the next step is. Maybe it
would also be worth it adding a /* get_user_pages_fast() takes the mmap_sem */
comment before the call ?
> ---
> drivers/media/platform/omap3isp/ispqueue.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispqueue.c
> b/drivers/media/platform/omap3isp/ispqueue.c index
> e15f01342058..bed380395e6c 100644
> --- a/drivers/media/platform/omap3isp/ispqueue.c
> +++ b/drivers/media/platform/omap3isp/ispqueue.c
> @@ -331,13 +331,9 @@ static int isp_video_buffer_prepare_user(struct
> isp_video_buffer *buf) if (buf->pages == NULL)
> return -ENOMEM;
>
> - down_read(¤t->mm->mmap_sem);
> - ret = get_user_pages(current, current->mm, data & PAGE_MASK,
> - buf->npages,
> - buf->vbuf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE, 0,
> - buf->pages, NULL);
> - up_read(¤t->mm->mmap_sem);
> -
> + ret = get_user_pages_fast(data & PAGE_MASK, buf->npages,
> + buf->vbuf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE,
> + buf->pages);
> if (ret != buf->npages) {
> buf->npages = ret < 0 ? 0 : ret;
> isp_video_buffer_cleanup(buf);
--
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
