On Mon, Oct 15, 2018 at 2:30 PM Thomas Hellstrom <thomas@xxxxxxxxxxxx> wrote: > > 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? The problem with adding that under e.g. CONFIG_PROVE_LOCKING or similar is that we have drivers violating it right now. And last time around this entire discussion came up no one wanted to do the work to convert drivers, since it means you get to add a slowpath to the most complex ioctl they have. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
