TTM Locking order of bo::reserve -> vm::mmap_sem

[Thomas Hellstrom]

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.


/Thomas


-------------- next part --------------
A non-text attachment was scrubbed...
Name: vm_lock.diff
Type: text/x-patch
Size: 1050 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131021/00152b51/attachment.bin>