From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: christian.koenig@amd.com, dri-devel@lists.freedesktop.org,
amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] locking/ww_mutex: cleanup lock->ctx usage in amdgpu
Date: Tue, 20 Feb 2018 13:31:14 +0100 [thread overview]
Message-ID: <9c0175bd-cd5e-e643-de9b-5b2f6b2449a1@gmail.com> (raw)
In-Reply-To: <20180220113318.GN22199@phenom.ffwll.local>
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.
next prev parent reply other threads:[~2018-02-20 12:31 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-15 14:19 [PATCH 1/3] locking/ww_mutex: cleanup lock->ctx usage in amdgpu Christian König
2018-02-15 14:19 ` [PATCH 2/3] drm/ttm: handle already locked BOs during eviction and swapout Christian König
2018-02-15 14:19 ` [PATCH 3/3] drm/ttm: keep BOs reserved until end of eviction Christian König
2018-02-15 20:36 ` [PATCH 1/3] locking/ww_mutex: cleanup lock->ctx usage in amdgpu Alex Deucher
2018-02-19 15:24 ` Daniel Vetter
2018-02-19 15:41 ` Christian König
2018-02-19 16:15 ` Daniel Vetter
2018-02-19 16:29 ` Christian König
2018-02-19 16:43 ` Daniel Vetter
2018-02-20 9:43 ` Christian König
2018-02-20 11:33 ` Daniel Vetter
2018-02-20 12:31 ` Christian König [this message]
2018-02-20 12:35 ` Peter Zijlstra
2018-02-20 13:08 ` Christian König
2018-02-20 13:22 ` Peter Zijlstra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9c0175bd-cd5e-e643-de9b-5b2f6b2449a1@gmail.com \
--to=ckoenig.leichtzumerken@gmail.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox