Re: [PATCH 1/3] locking/ww_mutex: cleanup lock->ctx usage in amdgpu

From: Christian KÃnig
Date: Tue Feb 20 2018 - 07:31:26 EST


Am 20.02.2018 um 12:33 schrieb Daniel Vetter:
[SNIP]
Ah, so the ttm_ctx I've spotted was something entirely different and
doesn't contain the ww_acquire_ctx (I didn't check)? I'd assume you have
the same ctx passed around to everything in ttm, but if that doesn't exist
then we can indeed not annotate ww_mutex_trylock_ctx with the right ctx.

Yes, exactly.

I actually tried this approach, e.g. put the ww_acquire_context into the ttm_operation_context and then use that with ww_mutex_trylock_ctx.

But a) that turned out to be to much hassle, e.g. at least amdgpu doesn't use a ww_acquire context in most cases.

And b) it actually wasn't what I was looking for, e.g. I couldn't distinct between the trylocked BOs an everything else any more.

[SNIP]
But to me it actually looks more like that makes it unnecessary complicated.
The use case in amdgpu which could only check the context isn't performance
critical.
Oh I'm not worried about the runtime overhead at all, I'm worried about
conceptual clarity of this stuff. If you have a ctx there's no need to
also look at ->owner.

Another idea: We drop the task argument from functions and go with the
following logic:

ww_mutex_is_owner(lock, ctx)
{
if (ctx)
return lock->ctx == ctx;
else
return lock->owner == current;
}

I think that would solve your use case, and gives us the neat interface
I'm aiming for. Kerneldoc can then explain what's happening for a NULL
ctx.

Good point, going to adjust the patches this way and resend.

Christian.