On 09/24/2013 11:43 AM, Maarten Lankhorst wrote:
Op 24-09-13 11:03, Thomas Hellstrom schreef:
On 09/24/2013 09:34 AM, Maarten Lankhorst wrote:
Op 24-09-13 09:22, Thomas Hellstrom schreef:
Actually, it's not the locking order bo:reserve -> mmap_sem that triggers the
lockdep warning, right? but the fact that copy_from_user could recurse into the
fault handler? Grabbing bo:reseves recursively? which should leave us free to
choose locking order at this point.
Same thing.
When PROVE_LOCKING is enabled the might_fault calls in copy_to/from_user do a
fake locking of mmap_sem, which means all locking errors, provided that the
reservation lock is taken at least once with mmap_sem held (eg the ttm fault
handler is called at least once, it can be done separately, it doesn't need to
be done in the same syscall). So any bugs will be found. The only thing to
worry about is that lockdep turns off after finding 1 error, so you have to
make sure it doesn't bomb out before completing tests, which is sometimes a
problem on early rc kernels. ;)
My point was that when we only have copy_[to|from]_user_inatomic inside any
bo:reservations, the might_fault would never be called inside any reservations
and we should, in principle, be free to choose locking order, provided of
course it could be done cleanly in fault()?
I think it makes sense to keep mmap_sem the outer lock here, and not do scary
things in fault. Especially if the mm locking is going to be changed in the
future. But you're correct, if holding reservations only inatomic should be
used.
~Maarten
Now that we after all were forced to enter the dark realm of copy-user
slowpaths, I don't really have a strong opinion anymore, other than that
we should try to avoid building too much code in that depends on either
locking order, so that we could change it if _really_ needed.
/Thomas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
