From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/i915: Remove impossibe checks from i915_gem_request_add_to_client
Date: Thu, 24 Mar 2016 09:49:25 +0000 [thread overview]
Message-ID: <56F3B825.2030502@linux.intel.com> (raw)
In-Reply-To: <20160323153159.GI21717@nuc-i3427.alporthouse.com>
On 23/03/16 15:31, Chris Wilson wrote:
> On Wed, Mar 23, 2016 at 03:15:03PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> There is only one caller of this functions which calls it under
>> strictly defined conditions. Error checking and return value
>> can be removed.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> BTW it looks like the whole point of this request list is to implement
>> the throttle ioctl? Looks like a strange ioctl with a fixed 20ms criteria,
>> could it be re-implemented somehow without having to have this list?
>>
>> Does it have to be per-client or could it wait for any requests on any
>> engine emitted 20ms ago?
>
> It is per-client, and I long wished the 20ms wasn't defined by the ABI.
Is it useful and/or widely used though?
With multiple clients submitting work I don't see the semantics are
deterministic. I was thinking if it could be replaced with a simpler
implementation for the similar random effect, losing the list altogether?
Especially wonder how the scheduler will affect it.
> What I proposed doing is this:
>
> https://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=breadcrumbs&id=e33947505b54582964c8c202b22f88fc5705f484
How is it safe? list_add modifies various pointers in multiple steps so
I didn't know that is safe against concurrent iteration.
RCU flavoured list API can do such things but in this case switching to
that would only enable lockless throttle and not gain anything on the
add_to_client path.
> Please note that this also fixes a race condition I've seen in the wild.
What is the race?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-03-24 9:49 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-23 15:15 [PATCH 1/2] drm/i915: Cache some registers in execlists hot paths Tvrtko Ursulin
2016-03-23 15:15 ` [PATCH 2/2] drm/i915: Remove impossibe checks from i915_gem_request_add_to_client Tvrtko Ursulin
2016-03-23 15:31 ` Chris Wilson
2016-03-24 9:49 ` Tvrtko Ursulin [this message]
2016-03-24 10:11 ` Chris Wilson
2016-03-23 16:06 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Cache some registers in execlists hot paths Patchwork
2016-03-24 11:16 ` [PATCH 1/2] " Chris Wilson
2016-03-24 11:31 ` Tvrtko Ursulin
2016-03-24 11:53 ` Chris Wilson
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=56F3B825.2030502@linux.intel.com \
--to=tvrtko.ursulin@linux.intel.com \
--cc=Intel-gfx@lists.freedesktop.org \
--cc=chris@chris-wilson.co.uk \
/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;
as well as URLs for NNTP newsgroup(s).