From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915/gt: Do not allow setting ring size for legacy ring submission
Date: Mon, 21 Jun 2021 14:49:21 +0200 [thread overview]
Message-ID: <cd5fec10-80b4-70d6-8b06-f852d9d1471c@linux.intel.com> (raw)
In-Reply-To: <04424cbf-750a-4cb9-a99a-6a5a3671452f@linux.intel.com>
Op 21-06-2021 om 14:08 schreef Tvrtko Ursulin:
>
> I had some questions on the trybot mailing list, let me copy&paste..
>
> On 21/06/2021 12:41, Maarten Lankhorst wrote:
>> It doesn't work for legacy ring submission, and is in the best case
>> ignored.
>
> Looks rejected instead of ignored:
>
> static int set_ringsize(struct i915_gem_context *ctx,
> struct drm_i915_gem_context_param *args)
> {
> if (!HAS_LOGICAL_RING_CONTEXTS(ctx->i915))
> return -ENODEV;
>>
>> In the worst case we end up freeing engine->legacy.ring for all other
>> active engines, resulting in a use-after-free.
>
> Worst case is cloning because ring_context_alloc is not taking a reference to engine->legacy.ring, or something else?
>
> Regards,
>
> Tvrtko
I only noticed this because tests started failing, if it should already hit -ENODEV then that's weird..
See: https://patchwork.freedesktop.org/series/91501/ for the failure.
It should not hit the INCOMPLETEs there.
The legacy contexts don't grab a reference to engine->legacy.ring, but a copy to the pointer, presumably because its lifetime is always shorter than the ring lifetime, so it will actually free it.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-06-21 12:49 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-21 11:41 [Intel-gfx] [PATCH 1/3] drm/i915/gt: Do not allow setting ring size for legacy ring submission Maarten Lankhorst
2021-06-21 11:41 ` [Intel-gfx] [PATCH 2/3] drm/i915: Use intel_context->pin_mutex only for context allocation Maarten Lankhorst
2021-06-21 11:41 ` [Intel-gfx] [PATCH 3/3] drm/i915: Remove intel_context->ops->(pre_pin/post_unpin) Maarten Lankhorst
2021-06-21 12:08 ` [Intel-gfx] [PATCH 1/3] drm/i915/gt: Do not allow setting ring size for legacy ring submission Tvrtko Ursulin
2021-06-21 12:49 ` Maarten Lankhorst [this message]
2021-06-21 12:52 ` Tvrtko Ursulin
2021-06-21 13:07 ` Maarten Lankhorst
2021-06-21 13:12 ` Tvrtko Ursulin
2021-06-21 13:20 ` Tvrtko Ursulin
2021-06-21 13:28 ` Maarten Lankhorst
2021-06-21 12:14 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] " Patchwork
2021-06-21 12:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-06-21 15:04 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
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=cd5fec10-80b4-70d6-8b06-f852d9d1471c@linux.intel.com \
--to=maarten.lankhorst@linux.intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
--cc=tvrtko.ursulin@linux.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