From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 17/21] drm/i915: Drop struct_mutex from around i915_retire_requests()
Date: Wed, 25 Sep 2019 09:49:48 +0100 [thread overview]
Message-ID: <ef7aaff6-a350-c2d9-df0b-b4e1856c5153@linux.intel.com> (raw)
In-Reply-To: <156940100781.4979.16490963525085766366@skylake-alporthouse-com>
On 25/09/2019 09:43, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-09-24 16:25:29)
>>
>> On 02/09/2019 05:02, Chris Wilson wrote:
>>> @@ -449,8 +447,6 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>>> struct i915_request *rq;
>>> int err;
>>>
>>> - lockdep_assert_held(&tl->gt->i915->drm.struct_mutex); /* lazy rq refs */
>>> -
>>> err = intel_timeline_pin(tl);
>>> if (err) {
>>> rq = ERR_PTR(err);
>>> @@ -461,10 +457,14 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>>> if (IS_ERR(rq))
>>> goto out_unpin;
>>>
>>> + i915_request_get(rq);
>>> +
>>> err = emit_ggtt_store_dw(rq, tl->hwsp_offset, value);
>>> i915_request_add(rq);
>>> - if (err)
>>> + if (err) {
>>> + i915_request_put(rq);
>>> rq = ERR_PTR(err);
>>> + }
>>>
>>> out_unpin:
>>> intel_timeline_unpin(tl);
>>> @@ -500,7 +500,6 @@ static int live_hwsp_engine(void *arg)
>>> struct intel_timeline **timelines;
>>> struct intel_engine_cs *engine;
>>> enum intel_engine_id id;
>>> - intel_wakeref_t wakeref;
>>> unsigned long count, n;
>>> int err = 0;
>>>
>>> @@ -515,14 +514,13 @@ static int live_hwsp_engine(void *arg)
>>> if (!timelines)
>>> return -ENOMEM;
>>>
>>> - mutex_lock(&i915->drm.struct_mutex);
>>> - wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>>> -
>>> count = 0;
>>> for_each_engine(engine, i915, id) {
>>> if (!intel_engine_can_store_dword(engine))
>>> continue;
>>>
>>> + intel_engine_pm_get(engine);
>>> +
>>> for (n = 0; n < NUM_TIMELINES; n++) {
>>> struct intel_timeline *tl;
>>> struct i915_request *rq;
>>> @@ -530,22 +528,26 @@ static int live_hwsp_engine(void *arg)
>>> tl = checked_intel_timeline_create(i915);
>>> if (IS_ERR(tl)) {
>>> err = PTR_ERR(tl);
>>> - goto out;
>>> + break;
>>> }
>>>
>>> rq = tl_write(tl, engine, count);
>>> if (IS_ERR(rq)) {
>>> intel_timeline_put(tl);
>>> err = PTR_ERR(rq);
>>> - goto out;
>>> + break;
>>> }
>>>
>>> timelines[count++] = tl;
>>> + i915_request_put(rq);
>>
>> This was a leak until now?
>
> No, we added a get into tl_write() so that we own a reference to the
> request, just so that ownership is clear across the waits (and it can't
> accidentally be retired).
/me scrolls up, scrolls down.. rubs eyes.. yep, I was blind.
[snip]
>> Essentially looks fine. Provisional, meaning keep it if you do some
>> small tweaks:
>
> Nothing seemed to be required.
Ack.
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:[~2019-09-25 8:49 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-02 4:02 [PATCH 01/21] drm/i915: Restrict the aliasing-ppgtt to the size of the ggtt Chris Wilson
2019-09-02 4:02 ` [PATCH 02/21] drm/i915: Report aliasing ppgtt size as ggtt size Chris Wilson
2019-09-02 8:55 ` Matthew Auld
2019-09-02 4:02 ` [PATCH 03/21] drm/i915/execlists: Ignore lost completion events Chris Wilson
2019-09-02 4:02 ` [PATCH 04/21] drm/i915: Refresh the errno to vmf_fault translations Chris Wilson
2019-09-03 15:34 ` Abdiel Janulgue
2019-09-02 4:02 ` [PATCH 05/21] drm/i915: Replace obj->pin_global with obj->frontbuffer Chris Wilson
2019-09-02 4:02 ` [PATCH 06/21] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
2019-09-02 4:02 ` [PATCH 07/21] drm/mm: Pack allocated/scanned boolean into a bitfield Chris Wilson
2019-09-02 4:02 ` [PATCH 08/21] drm/i915: Make shrink/unshrink be atomic Chris Wilson
2019-09-10 19:54 ` Matthew Auld
2019-09-02 4:02 ` [PATCH 09/21] drm/i915: Only track bound elements of the GTT Chris Wilson
2019-09-02 4:02 ` [PATCH 10/21] drm/i915: Make i915_vma.flags atomic_t for mutex reduction Chris Wilson
2019-09-10 20:06 ` Matthew Auld
2019-09-02 4:02 ` [PATCH 11/21] drm/i915/gtt: Make sure the gen6 ppgtt is bound before first use Chris Wilson
2019-09-10 20:17 ` Matthew Auld
2019-09-02 4:02 ` [PATCH 12/21] drm/i915: Mark up address spaces that may need to allocate Chris Wilson
2019-09-20 16:22 ` Tvrtko Ursulin
2019-09-20 16:35 ` Chris Wilson
2019-09-23 8:10 ` Tvrtko Ursulin
2019-09-25 8:23 ` Chris Wilson
2019-09-25 15:59 ` Tvrtko Ursulin
2019-09-27 17:03 ` Chris Wilson
2019-09-02 4:02 ` [PATCH 13/21] drm/i915: Pull i915_vma_pin under the vm->mutex Chris Wilson
2019-09-16 10:13 ` Tvrtko Ursulin
2019-09-16 11:10 ` Chris Wilson
2019-09-17 12:37 ` Tvrtko Ursulin
2019-09-17 18:56 ` Chris Wilson
2019-09-19 13:37 ` Tvrtko Ursulin
2019-09-19 14:05 ` Chris Wilson
2019-09-02 4:02 ` [PATCH 14/21] drm/i915: Push the i915_active.retire into a worker Chris Wilson
2019-09-02 4:02 ` [PATCH 15/21] drm/i915: Coordinate i915_active with its own mutex Chris Wilson
2019-09-20 16:14 ` Tvrtko Ursulin
2019-09-20 16:32 ` Chris Wilson
2019-09-02 4:02 ` [PATCH 16/21] drm/i915: Move idle barrier cleanup into engine-pm Chris Wilson
2019-09-20 16:18 ` Tvrtko Ursulin
2019-09-02 4:02 ` [PATCH 17/21] drm/i915: Drop struct_mutex from around i915_retire_requests() Chris Wilson
2019-09-24 15:25 ` Tvrtko Ursulin
2019-09-25 8:43 ` Chris Wilson
2019-09-25 8:49 ` Tvrtko Ursulin [this message]
2019-09-02 4:03 ` [PATCH 18/21] drm/i915: Remove the GEM idle worker Chris Wilson
2019-09-24 15:26 ` Tvrtko Ursulin
2019-09-02 4:03 ` [PATCH 19/21] drm/i915: Merge wait_for_timelines with retire_request Chris Wilson
2019-09-24 15:57 ` Tvrtko Ursulin
2019-09-25 8:54 ` Chris Wilson
2019-09-02 4:03 ` [PATCH 20/21] drm/i915: Move request runtime management onto gt Chris Wilson
2019-09-02 4:03 ` [PATCH 21/21] drm/i915: Move global activity tracking from GEM to GT Chris Wilson
2019-09-25 9:55 ` Tvrtko Ursulin
2019-09-02 4:54 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/21] drm/i915: Restrict the aliasing-ppgtt to the size of the ggtt Patchwork
2019-09-02 5:20 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-02 8:00 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-09-02 8:52 ` [PATCH 01/21] " Matthew Auld
-- strict thread matches above, loose matches on Subject: below --
2019-08-30 6:11 [PATCH 01/21] drm/i915/gtt: Downgrade Baytrail back to aliasing-ppgtt Chris Wilson
2019-08-30 6:12 ` [PATCH 17/21] drm/i915: Drop struct_mutex from around i915_retire_requests() 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=ef7aaff6-a350-c2d9-df0b-b4e1856c5153@linux.intel.com \
--to=tvrtko.ursulin@linux.intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.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