Possible lock inversion in ttm_bo_vm_access

[Thomas Hellstrom]

On 10/15/2018 12:55 PM, Koenig, Christian wrote:
> Am 15.10.2018 um 12:42 schrieb Thomas Hellstrom:
>> On 10/15/2018 10:43 AM, Koenig, Christian wrote:
>>> Hi Thomas,
>>>
>>>> 1. You need the a lock with the order lock->mmap_sem, cause otherwise
>>>> you run into trouble when drm_vma_node_unmap() needs to be called.
>>>> Why is this? unmap_mapping_range() never takes the mmap_sem. It takes
>>>> a finer-level per-address-space lock.
>>> Well, that is interesting. Where do we actually then have the order of
>>> reservation->mmap_sem defined?
>> TTM doesn't. But some drivers using TTM do, which prompted the wording
>> in the fault handler comments.
>> But as Daniel mentioned, to be able to share buffers throughout DRM,
>> we at some point need to agree on a locking order, and when we do it's
>> important that we consider all relevant use-cases and workarounds.
>>
>> To me it certainly looks like the vm_ops implementations would be more
>> straightforward if we were to use mmap_sem->bo_reserve, but that would
>> again require a number of drivers to rework their copy_*_user code.
>> If those drivers, however, insisted on keeping their copy_*_user code,
>> they probably need to fix the recursive bo reservation up anyway, for
>> example by making sure not to copy from a vma that has VM_IO set.
> Yeah, completely agree. If you ask me using copy_(from|to)_user while a
> BO is reserved is currently completely forbidden.

So if we all agree that this should be completely forbidden, and this is 
what requires the bo_reserve->mmap_sem locking order, then I guess the 
logical follow up is trying to enforce mmap_sem->bo_reserve and pursuade 
driver maintainers to convert. Perhaps as a configurable lockdep option?

[SNIP]

/Thomas