Op 27-05-13 13:15, Peter Zijlstra schreef:
On Mon, May 27, 2013 at 12:52:00PM +0200, Maarten Lankhorst wrote:
The reason ttm needed it was because there was another lock that interacted with the ctx lock in a weird way. The ww lock it was using was inverted with another lock, so it had to grab that lock first, perform a trylock on the ww lock, and if that failed unlock the lock, wait for it to be unlocked, then retry the same thing again. I'm so glad I managed to fix that mess, if you really need ww_mutex_trylock with a ctx, it's an indication your locking is wrong.
For ww_mutex_trylock with a context to be of any use you would also need to return 0 or a -errno, (-EDEADLK, -EBUSY (already locked by someone else), or -EALREADY). This would make the trylock very different from other trylocks, and very confusing because if (ww_mutex_trylock(lock, ctx)) would not do what you would think it would do.
Yuck ;-)
Anyway, what I was thinking of is something like:
T0 T1
try A lock B lock B lock A
Now, if for some reason T1 won the lottery such that T0 would have to be wounded, T0's context would indicate its the first entry and not return -EDEADLK.
And this sounds like something lockdep is designed to complain about.
Nothing stops you from doing try A then doing try B, which would be the correct way to deal with this situation. Why would you trylock one, and then not do the same for another?
OTOH, anybody doing creative things like that might well deserve whatever they get ;-)
Indeed!
The thing is; if there could exist something like:
ww_mutex_trylock(struct ww_mutex *, struct ww_acquire_ctx *ctx);
Then we should not now take away that name and make it mean something else; namely: ww_mutex_trylock_single().
Unless we want to allow .ctx=NULL to mean _single.
As to why I proposed that (.ctx=NULL meaning _single); I suppose because I'm a minimalist at heart.
Minimalism isn't bad, it's just knowing when to sto
:-)