All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: "Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>,
	Intel-gfx@lists.freedesktop.org
Cc: Andrzej Hajda <andrzej.hajda@intel.com>,
	dri-devel@lists.freedesktop.org, stable@vger.kernel.org
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/guc: Fix revocation of non-persistent contexts
Date: Wed, 5 Oct 2022 08:59:16 +0100	[thread overview]
Message-ID: <7af80924-73cf-14fb-44d4-b5ed28bbdc9f@linux.intel.com> (raw)
In-Reply-To: <e2140d7a-b084-4298-d92a-649d0672fcc7@intel.com>


On 04/10/2022 16:13, Ceraolo Spurio, Daniele wrote:
> On 10/4/2022 4:14 AM, Tvrtko Ursulin wrote:
>>
>> On 03/10/2022 13:16, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> Patch which added graceful exit for non-persistent contexts missed the
>>> fact it is not enough to set the exiting flag on a context and let the
>>> backend handle it from there.
>>>
>>> GuC backend cannot handle it because it runs independently in the
>>> firmware and driver might not see the requests ever again. Patch also
>>> missed the fact some usages of intel_context_is_banned in the GuC 
>>> backend
>>> needed replacing with newly introduced intel_context_is_schedulable.
>>>
>>> Fix the first issue by calling into backend revoke when we know this is
>>> the last chance to do it. Fix the second issue by replacing
>>> intel_context_is_banned with intel_context_is_schedulable, which should
>>> always be safe since latter is a superset of the former.
>>>
>>> v2:
>>>   * Just call ce->ops->revoke unconditionally. (Andrzej)
>>
>> CI is happy - could I get some acks for the GuC backend changes please?
> 
> I think we still need to have a longer conversation on the revoking 
> times, but in the meantime this fixes the immediate concerns, so:
> 
> Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Thanks, I've pushed it so should unbreak 6.0 via stable.

For follow up work I am okay either with a fixes 20ms timeout (this was 
enough for users which originally reported it), or go with fully 
configurable? Latter feels a bit over the top since it would then me a 
kconfig and sysfs to align with the normal preempt timeout.

Regards,

Tvrtko

WARNING: multiple messages have this Message-ID (diff)
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: "Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>,
	Intel-gfx@lists.freedesktop.org
Cc: stable@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	John Harrison <John.C.Harrison@Intel.com>
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/guc: Fix revocation of non-persistent contexts
Date: Wed, 5 Oct 2022 08:59:16 +0100	[thread overview]
Message-ID: <7af80924-73cf-14fb-44d4-b5ed28bbdc9f@linux.intel.com> (raw)
In-Reply-To: <e2140d7a-b084-4298-d92a-649d0672fcc7@intel.com>


On 04/10/2022 16:13, Ceraolo Spurio, Daniele wrote:
> On 10/4/2022 4:14 AM, Tvrtko Ursulin wrote:
>>
>> On 03/10/2022 13:16, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> Patch which added graceful exit for non-persistent contexts missed the
>>> fact it is not enough to set the exiting flag on a context and let the
>>> backend handle it from there.
>>>
>>> GuC backend cannot handle it because it runs independently in the
>>> firmware and driver might not see the requests ever again. Patch also
>>> missed the fact some usages of intel_context_is_banned in the GuC 
>>> backend
>>> needed replacing with newly introduced intel_context_is_schedulable.
>>>
>>> Fix the first issue by calling into backend revoke when we know this is
>>> the last chance to do it. Fix the second issue by replacing
>>> intel_context_is_banned with intel_context_is_schedulable, which should
>>> always be safe since latter is a superset of the former.
>>>
>>> v2:
>>>   * Just call ce->ops->revoke unconditionally. (Andrzej)
>>
>> CI is happy - could I get some acks for the GuC backend changes please?
> 
> I think we still need to have a longer conversation on the revoking 
> times, but in the meantime this fixes the immediate concerns, so:
> 
> Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Thanks, I've pushed it so should unbreak 6.0 via stable.

For follow up work I am okay either with a fixes 20ms timeout (this was 
enough for users which originally reported it), or go with fully 
configurable? Latter feels a bit over the top since it would then me a 
kconfig and sysfs to align with the normal preempt timeout.

Regards,

Tvrtko

WARNING: multiple messages have this Message-ID (diff)
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: "Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>,
	Intel-gfx@lists.freedesktop.org
Cc: Andrzej Hajda <andrzej.hajda@intel.com>,
	dri-devel@lists.freedesktop.org, stable@vger.kernel.org,
	John Harrison <John.C.Harrison@Intel.com>
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/guc: Fix revocation of non-persistent contexts
Date: Wed, 5 Oct 2022 08:59:16 +0100	[thread overview]
Message-ID: <7af80924-73cf-14fb-44d4-b5ed28bbdc9f@linux.intel.com> (raw)
In-Reply-To: <e2140d7a-b084-4298-d92a-649d0672fcc7@intel.com>


On 04/10/2022 16:13, Ceraolo Spurio, Daniele wrote:
> On 10/4/2022 4:14 AM, Tvrtko Ursulin wrote:
>>
>> On 03/10/2022 13:16, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> Patch which added graceful exit for non-persistent contexts missed the
>>> fact it is not enough to set the exiting flag on a context and let the
>>> backend handle it from there.
>>>
>>> GuC backend cannot handle it because it runs independently in the
>>> firmware and driver might not see the requests ever again. Patch also
>>> missed the fact some usages of intel_context_is_banned in the GuC 
>>> backend
>>> needed replacing with newly introduced intel_context_is_schedulable.
>>>
>>> Fix the first issue by calling into backend revoke when we know this is
>>> the last chance to do it. Fix the second issue by replacing
>>> intel_context_is_banned with intel_context_is_schedulable, which should
>>> always be safe since latter is a superset of the former.
>>>
>>> v2:
>>>   * Just call ce->ops->revoke unconditionally. (Andrzej)
>>
>> CI is happy - could I get some acks for the GuC backend changes please?
> 
> I think we still need to have a longer conversation on the revoking 
> times, but in the meantime this fixes the immediate concerns, so:
> 
> Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Thanks, I've pushed it so should unbreak 6.0 via stable.

For follow up work I am okay either with a fixes 20ms timeout (this was 
enough for users which originally reported it), or go with fully 
configurable? Latter feels a bit over the top since it would then me a 
kconfig and sysfs to align with the normal preempt timeout.

Regards,

Tvrtko

  reply	other threads:[~2022-10-05  7:59 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-30  9:47 [Intel-gfx] [PATCH] drm/i915/guc: Fix revocation of non-persistent contexts Tvrtko Ursulin
2022-09-30  9:47 ` Tvrtko Ursulin
2022-09-30 11:08 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2022-09-30 14:52 ` [Intel-gfx] [PATCH] " Andrzej Hajda
2022-10-03  7:59   ` Tvrtko Ursulin
2022-10-03  9:30     ` Andrzej Hajda
2022-10-01  7:57 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork
2022-10-03 12:16 ` [Intel-gfx] [PATCH v2] " Tvrtko Ursulin
2022-10-03 12:16   ` Tvrtko Ursulin
2022-10-03 12:16   ` Tvrtko Ursulin
2022-10-04 11:14   ` [Intel-gfx] " Tvrtko Ursulin
2022-10-04 11:14     ` Tvrtko Ursulin
2022-10-04 11:14     ` Tvrtko Ursulin
2022-10-04 15:13     ` Ceraolo Spurio, Daniele
2022-10-04 15:13       ` Ceraolo Spurio, Daniele
2022-10-04 15:13       ` Ceraolo Spurio, Daniele
2022-10-05  7:59       ` Tvrtko Ursulin [this message]
2022-10-05  7:59         ` Tvrtko Ursulin
2022-10-05  7:59         ` Tvrtko Ursulin
2022-10-03 13:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/guc: Fix revocation of non-persistent contexts (rev2) Patchwork
2022-10-03 15:51 ` [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=7af80924-73cf-14fb-44d4-b5ed28bbdc9f@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=Intel-gfx@lists.freedesktop.org \
    --cc=andrzej.hajda@intel.com \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=stable@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.