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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
