videobuf mmap deadlock

videobuf mmap deadlock

[Pete Eberlein]

The patch "videobuf_vm_{open,close} race fixes" 
https://linuxtv.org/patch/18365/ introduced a deadlock in 3.11.

My driver uses videobuf_vmalloc initialized with ext_lock set to NULL.
My driver's mmap function calls videobuf_mmap_mapper
videobuf_mmap_mapper calls videobuf_queue_lock on q
videobuf_mmap_mapper calls  __videobuf_mmap_mapper
  __videobuf_mmap_mapper calls videobuf_vm_open
videobuf_vm_open calls videobuf_queue_lock on q (introduced by above patch)
deadlocked

This is not an issue if ext_lock is non-NULL, since videobuf_queue_lock 
is a no-op in that case.

Did I do something wrong, or is this a valid regression?

Regards,
Pete Eberlein

Re: videobuf mmap deadlock

[Hans Verkuil]

Hi Pete,

On 11/01/2013 07:54 PM, Pete Eberlein wrote:
> The patch "videobuf_vm_{open,close} race fixes" 
> https://linuxtv.org/patch/18365/ introduced a deadlock in 3.11.
> 
> My driver uses videobuf_vmalloc initialized with ext_lock set to NULL.

Which driver are we talking about?

> My driver's mmap function calls videobuf_mmap_mapper
> videobuf_mmap_mapper calls videobuf_queue_lock on q
> videobuf_mmap_mapper calls  __videobuf_mmap_mapper
>   __videobuf_mmap_mapper calls videobuf_vm_open
> videobuf_vm_open calls videobuf_queue_lock on q (introduced by above patch)
> deadlocked
> 
> This is not an issue if ext_lock is non-NULL, since videobuf_queue_lock 
> is a no-op in that case.
> 
> Did I do something wrong, or is this a valid regression?

I think this is a valid regression. Using an atomic_t for the count is a better
solution IMHO. Anyone up for writing a patch?

Locking in videobuf is very messy and I recommend moving to videobuf2 which is
much more reliable and much easier to understand as well.

Regards,

	Hans