Re: [PATCH] uvcvideo: Don't call vb2 mmap and get_unmapped_area with queue lock held
Hi Bjørn, (it took me half an hour to figure out how to write ø on my keyboard :-)) On Monday 09 March 2015 12:06:36 Bjørn Mork wrote: > Bjørn Mork writes: > > Laurent Pinchart writes: > >> Bjørn, does this fix the circular locking dependency you have reported in > >> "[v3.19-rc7] possible circular locking dependency in uvc_queue_streamoff" > >> ? The report mentions involves locks, so I'm not 100% this patch will fix > >> the issue. > > > > Sorry, I forgot all about that report after firing it off... Should > > have followed it up with some more details. > > > > Grepping my logs now I cannot find this warning at all after the one I > > reported. I see it once before (while running 3.19-rc6). So it is > > definitely not easily reproducible. And I have a bad feeling the > > trigger might involve completely unrelated USB issues... > > > > In any case, thanks for the patch. I will test it for a while and let > > you know if the same warning shows ut with it. But based on the rare > > occurence, I don't think I ever will be able to positively confirm that > > the warning is gone. > > FWIW, I have not seen the warning after applying this patch, so it > appears to fix the problem. Thanks. You're welcome. > If I'm wrong, then I'm sure Murphy will tell us as soon as I send this > email :-) I'd be happy to prove Murphy wrong for once. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] uvcvideo: Don't call vb2 mmap and get_unmapped_area with queue lock held
Bjørn Mork writes: > Laurent Pinchart writes: > >> Bjørn, does this fix the circular locking dependency you have reported in >> "[v3.19-rc7] possible circular locking dependency in uvc_queue_streamoff" ? >> The report mentions involves locks, so I'm not 100% this patch will fix the >> issue. > > Sorry, I forgot all about that report after firing it off... Should > have followed it up with some more details. > > Grepping my logs now I cannot find this warning at all after the one I > reported. I see it once before (while running 3.19-rc6). So it is > definitely not easily reproducible. And I have a bad feeling the > trigger might involve completely unrelated USB issues... > > In any case, thanks for the patch. I will test it for a while and let > you know if the same warning shows ut with it. But based on the rare > occurence, I don't think I ever will be able to positively confirm that > the warning is gone. FWIW, I have not seen the warning after applying this patch, so it appears to fix the problem. Thanks. If I'm wrong, then I'm sure Murphy will tell us as soon as I send this email :-) Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] uvcvideo: Don't call vb2 mmap and get_unmapped_area with queue lock held
Laurent Pinchart writes: > Bjørn, does this fix the circular locking dependency you have reported in > "[v3.19-rc7] possible circular locking dependency in uvc_queue_streamoff" ? > The report mentions involves locks, so I'm not 100% this patch will fix the > issue. Sorry, I forgot all about that report after firing it off... Should have followed it up with some more details. Grepping my logs now I cannot find this warning at all after the one I reported. I see it once before (while running 3.19-rc6). So it is definitely not easily reproducible. And I have a bad feeling the trigger might involve completely unrelated USB issues... In any case, thanks for the patch. I will test it for a while and let you know if the same warning shows ut with it. But based on the rare occurence, I don't think I ever will be able to positively confirm that the warning is gone. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] uvcvideo: Don't call vb2 mmap and get_unmapped_area with queue lock held
videobuf2 has long been subject to AB-BA style deadlocks due to the
queue lock and mmap_sem being taken in different orders for the mmap and
get_unmapped_area operations. The problem has been fixed by making those
two operations callable without taking the queue lock, using an
mmap_lock internal to videobuf2.
The uvcvideo driver still calls the mmap and get_unmapped_area
operations with the queue lock held, resulting in a potential deadlock.
As the operations can now be called without locking the queue, fix it.
Reported-by: Bjørn Mork
Signed-off-by: Laurent Pinchart
---
drivers/media/usb/uvc/uvc_queue.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
Bjørn, does this fix the circular locking dependency you have reported in
"[v3.19-rc7] possible circular locking dependency in uvc_queue_streamoff" ?
The report mentions involves locks, so I'm not 100% this patch will fix the
issue.
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index 10c554e..87a19f3 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -306,25 +306,14 @@ int uvc_queue_streamoff(struct uvc_video_queue *queue,
enum v4l2_buf_type type)
int uvc_queue_mmap(struct uvc_video_queue *queue, struct vm_area_struct *vma)
{
- int ret;
-
- mutex_lock(&queue->mutex);
- ret = vb2_mmap(&queue->queue, vma);
- mutex_unlock(&queue->mutex);
-
- return ret;
+ return vb2_mmap(&queue->queue, vma);
}
#ifndef CONFIG_MMU
unsigned long uvc_queue_get_unmapped_area(struct uvc_video_queue *queue,
unsigned long pgoff)
{
- unsigned long ret;
-
- mutex_lock(&queue->mutex);
- ret = vb2_get_unmapped_area(&queue->queue, 0, 0, pgoff, 0);
- mutex_unlock(&queue->mutex);
- return ret;
+ return vb2_get_unmapped_area(&queue->queue, 0, 0, pgoff, 0);
}
#endif
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
