From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: John.C.Harrison@Intel.com, Intel-GFX@Lists.FreeDesktop.Org
Subject: Re: [RFC 7/9] drm/i915: Interrupt driven fences
Date: Mon, 20 Jul 2015 11:09:04 +0200 [thread overview]
Message-ID: <55ACBAB0.6090408@linux.intel.com> (raw)
In-Reply-To: <1437143483-6234-8-git-send-email-John.C.Harrison@Intel.com>
Op 17-07-15 om 16:31 schreef John.C.Harrison@Intel.com:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> The intended usage model for struct fence is that the signalled status should be
> set on demand rather than polled. That is, there should not be a need for a
> 'signaled' function to be called everytime the status is queried. Instead,
> 'something' should be done to enable a signal callback from the hardware which
> will update the state directly. In the case of requests, this is the seqno
> update interrupt. The idea is that this callback will only be enabled on demand
> when something actually tries to wait on the fence.
>
> This change removes the polling test and replaces it with the callback scheme.
> Each fence is added to a 'please poke me' list at the start of
> i915_add_request(). The interrupt handler then scans through the 'poke me' list
> when a new seqno pops out and signals any matching fence/request. The fence is
> then removed from the list so the entire request stack does not need to be
> scanned every time. Note that the fence is added to the list before the commands
> to generate the seqno interrupt are added to the ring. Thus the sequence is
> guaranteed to be race free if the interrupt is already enabled.
>
> Note that the interrupt is only enabled on demand (i.e. when __wait_request() is
> called). Thus there is still a potential race when enabling the interrupt as the
> request may already have completed. However, this is simply solved by calling
> the interrupt processing code immediately after enabling the interrupt and
> thereby checking for already completed requests.
This race will happen on any enable_signaling implementation, just something to be aware of.
> Lastly, the ring clean up code has the possibility to cancel outstanding
> requests (e.g. because TDR has reset the ring). These requests will never get
> signalled and so must be removed from the signal list manually. This is done by
> setting a 'cancelled' flag and then calling the regular notify/retire code path
> rather than attempting to duplicate the list manipulatation and clean up code in
> multiple places. This also avoid any race condition where the cancellation
> request might occur after/during the completion interrupt actually arriving.
I notice in this commit you only clean up requests when the refcount drops to 0.
What resources need to be kept after the fence is signaled? Can't you queue the
delayed work from the function that signals the fence and make sure it holds a
refcount?
I'm curious because if userspace can grab a reference for a fence it might lead
to not freeing resources for a long time..
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-07-20 9:09 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-17 14:31 [RFC 0/9] Convert requests to use struct fence John.C.Harrison
2015-07-17 14:31 ` [RFC 1/9] staging/android/sync: Support sync points created from dma-fences John.C.Harrison
2015-07-17 14:44 ` Tvrtko Ursulin
2015-07-17 14:31 ` [RFC 2/9] android: add sync_fence_create_dma John.C.Harrison
2015-07-17 14:31 ` [RFC 3/9] drm/i915: Convert requests to use struct fence John.C.Harrison
2015-07-21 7:05 ` Daniel Vetter
2015-07-28 10:01 ` John Harrison
2015-07-22 14:26 ` Tvrtko Ursulin
2015-07-28 10:10 ` John Harrison
2015-08-03 9:17 ` Tvrtko Ursulin
2015-07-22 14:45 ` Tvrtko Ursulin
2015-07-28 10:18 ` John Harrison
2015-08-03 9:18 ` Tvrtko Ursulin
2015-07-17 14:31 ` [RFC 4/9] drm/i915: Removed now redudant parameter to i915_gem_request_completed() John.C.Harrison
2015-07-17 14:31 ` [RFC 5/9] drm/i915: Add per context timelines to fence object John.C.Harrison
2015-07-23 13:50 ` Tvrtko Ursulin
2015-10-28 12:59 ` John Harrison
2015-11-17 13:54 ` Daniel Vetter
2015-07-17 14:31 ` [RFC 6/9] drm/i915: Delay the freeing of requests until retire time John.C.Harrison
2015-07-23 14:25 ` Tvrtko Ursulin
2015-10-28 13:00 ` John Harrison
2015-10-28 13:42 ` Tvrtko Ursulin
2015-07-17 14:31 ` [RFC 7/9] drm/i915: Interrupt driven fences John.C.Harrison
2015-07-20 9:09 ` Maarten Lankhorst [this message]
2015-07-21 7:19 ` Daniel Vetter
2015-07-27 11:33 ` Tvrtko Ursulin
2015-10-28 13:00 ` John Harrison
2015-07-27 13:20 ` Tvrtko Ursulin
2015-07-27 14:00 ` Daniel Vetter
2015-08-03 9:20 ` Tvrtko Ursulin
2015-08-05 8:05 ` Daniel Vetter
2015-08-05 11:05 ` Maarten Lankhorst
2015-07-17 14:31 ` [RFC 8/9] drm/i915: Updated request structure tracing John.C.Harrison
2015-07-17 14:31 ` [RFC 9/9] drm/i915: Add sync framework support to execbuff IOCTL John.C.Harrison
2015-07-27 13:00 ` Tvrtko Ursulin
2015-10-28 13:01 ` John Harrison
2015-10-28 14:31 ` Tvrtko Ursulin
2015-11-17 13:59 ` Daniel Vetter
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=55ACBAB0.6090408@linux.intel.com \
--to=maarten.lankhorst@linux.intel.com \
--cc=Intel-GFX@Lists.FreeDesktop.Org \
--cc=John.C.Harrison@Intel.com \
/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