On 10/21/2013 11:01 AM, Maarten Lankhorst wrote: > op 21-10-13 10:48, Thomas Hellstrom schreef: >> Hi! >> >> As discussed previously the current locking order in TTM of these locks is >> bo::reserve -> vm::mmap_sem. This leads to a hack in >> the TTM fault() handle to try and revert the locking order. If a tryreserve >> failed, we tried to have the vm code release the mmap_sem() and then >> schedule, to give the holder of bo::reserve a chance to release the lock. >> This solution is no longer legal, since we've been more or less kindly asked >> to remove the set_need_resched() call. >> >> Maarten has proposed to invert the locking order. I've previously said I had >> no strong preference. The current locking order dates back from the time >> when TTM wasn't using unmap_mapping_range() but walked the page tables >> itself, updating PTEs as needed. Furthermore it was needed for user bos that >> used get_user_pages() in the TTM populate and swap-in methods. User-bos were >> removed some time ago but I'm looking at re-adding them. They would suite >> the VMware model of cached-only pages very well. I see uses both in the >> gallium API, XA's DMA functionality and openCL. >> >> We would then need a somewhat nicer way to invert the locking order. I've >> attached a solution that ups the mmap_sem and then reserves, but due to how >> the fault API is done, we then need to release the reserve and retry the >> fault. This of course opens up for starvation, but I don't think starvation >> at this point is very likely: One thread being refused to write or read from >> a buffer object because the GPU is continously busy with it. If this *would* >> become a problem, it's probably possible to modify the fault code to allow >> us to hold locks until the retried fault, but that would be a bit invasive, >> since it touches the arch code.... >> >> Basically I'm proposing to keep the current locking order. > I'm not sure why we have to worry about mmap_sem lock being taken before > bo::reserve. If we already hold mmap_sem, > no extra locking is needed for get_user_pages.
Typically, they are populated outside of fault, as part of execbuf, where we don't hold and don't want to hold mmap_sem(). In fact, user bo's should not be remappable through the TTM VM system. Anyway, we need to grab the mmap_sem inside ttm_populate for user buffers. > Releasing it is a bit silly. I think we should keep mmap_sem as outer > lock, and have bo::reserve as inner, even if it might complicate support for > user-bo's. I'm not sure what you can do > with user-bo's that can't be done by allocating the same bo from kernel first > and map + populate it. > > ~Maarten Using DMA API analogy, user BOs correspond to using streaming DMA whereas normal BOs correspond to alloced DMA memory buffers. We boost performance and save resources. Thanks, /Thomas > > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
