[PATCH] drm/i915: Mark pending batches correctly on reset

[PATCH] drm/i915: Mark pending batches correctly on reset

[Mika Kuoppala]

For contexts that get their requests NOPed after a reset,
correctly count them as pending.

Testcase: igt/tests/gem_reset_stats
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 87018df9..e025542 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2615,6 +2615,7 @@ static void reset_request(struct drm_i915_gem_request *request)
 		head = 0;
 	}
 	memset(vaddr + head, 0, request->postfix - head);
+	i915_set_reset_status(request->ctx, false);
 }
 
 static void i915_gem_reset_engine(struct intel_engine_cs *engine)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Mark pending batches correctly on reset

[Chris Wilson]

On Wed, Oct 26, 2016 at 03:07:59PM +0300, Mika Kuoppala wrote:
> For contexts that get their requests NOPed after a reset,
> correctly count them as pending.
> 
> Testcase: igt/tests/gem_reset_stats
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

We agreed that this was an incorrect interpretation of the robustness
api, that neither handles tdr nor scales to multiple timlines.

Currently, we only mark as innocent the contexts/batch executing on the
hw on the good rings at the time of the reset.

>  drivers/gpu/drm/i915/i915_gem.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 87018df9..e025542 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2615,6 +2615,7 @@ static void reset_request(struct drm_i915_gem_request *request)
>  		head = 0;
>  	}
>  	memset(vaddr + head, 0, request->postfix - head);
> +	i915_set_reset_status(request->ctx, false);

This does not handle all the multiple timelines we have that are not
even submitted yet.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Mark pending batches correctly on reset

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Wed, Oct 26, 2016 at 03:07:59PM +0300, Mika Kuoppala wrote:
>> For contexts that get their requests NOPed after a reset,
>> correctly count them as pending.
>> 
>> Testcase: igt/tests/gem_reset_stats
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>
> We agreed that this was an incorrect interpretation of the robustness
> api, that neither handles tdr nor scales to multiple timlines.
>

I remember agreeing with the active one atleast. Perhaps being
ignorant on the multiple timelines case.

Is the reasoning here that there is no actual benefit of marking
batches pending as it is superflous in replay case. In another
words, the distinction between batch being queued before
submission and after, is a moot from userspace point of view?

> Currently, we only mark as innocent the contexts/batch executing on the
> hw on the good rings at the time of the reset.
>

I am ok with this. The interpretation of 'pending' changes but it
is more meaningful if one thinks pending on hardware.

-Mika

>>  drivers/gpu/drm/i915/i915_gem.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 87018df9..e025542 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -2615,6 +2615,7 @@ static void reset_request(struct drm_i915_gem_request *request)
>>  		head = 0;
>>  	}
>>  	memset(vaddr + head, 0, request->postfix - head);
>> +	i915_set_reset_status(request->ctx, false);
>
> This does not handle all the multiple timelines we have that are not
> even submitted yet.
> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Mark pending batches correctly on reset

[Chris Wilson]

On Wed, Oct 26, 2016 at 03:32:20PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > On Wed, Oct 26, 2016 at 03:07:59PM +0300, Mika Kuoppala wrote:
> >> For contexts that get their requests NOPed after a reset,
> >> correctly count them as pending.
> >> 
> >> Testcase: igt/tests/gem_reset_stats
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> >
> > We agreed that this was an incorrect interpretation of the robustness
> > api, that neither handles tdr nor scales to multiple timlines.
> >
> 
> I remember agreeing with the active one atleast. Perhaps being
> ignorant on the multiple timelines case.
> 
> Is the reasoning here that there is no actual benefit of marking
> batches pending as it is superflous in replay case. In another
> words, the distinction between batch being queued before
> submission and after, is a moot from userspace point of view?

Yes. And it gives them information that they are not otherwise privy to.
 
> > Currently, we only mark as innocent the contexts/batch executing on the
> > hw on the good rings at the time of the reset.
> >
> 
> I am ok with this. The interpretation of 'pending' changes but it
> is more meaningful if one thinks pending on hardware.

That's my understanding as well. We only mark the affected batches -
either it is guilty and scrapped, or it is innocent and rerun. But we
may still see corruption in the innocent batch (as it may change state
internally but the initial state is not restored upon reset). Everyone
else should not be affected (there is always some dependencies as
corruption may propagate, but we do identify the root).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for drm/i915: Mark pending batches correctly on reset

[Patchwork]

== Series Details ==

Series: drm/i915: Mark pending batches correctly on reset
URL   : https://patchwork.freedesktop.org/series/14403/
State : warning

== Summary ==

Series 14403v1 drm/i915: Mark pending batches correctly on reset
https://patchwork.freedesktop.org/api/1.0/series/14403/revisions/1/mbox/

Test drv_module_reload_basic:
                pass       -> DMESG-WARN (fi-skl-6770hq)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:0   skip:61 
fi-ivb-3520m     total:246  pass:220  dwarn:0   dfail:0   fail:0   skip:26 
fi-ivb-3770      total:246  pass:220  dwarn:0   dfail:0   fail:0   skip:26 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6770hq    total:246  pass:231  dwarn:1   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 
fi-snb-2600      total:246  pass:208  dwarn:0   dfail:0   fail:0   skip:38 

760634d2b78657f0a2267f73bff0d527f6c69ce5 drm-intel-nightly: 2016y-10m-26d-11h-52m-53s UTC integration manifest
37ba9f0 drm/i915: Mark pending batches correctly on reset

Full results at https://intel-gfx-ci.01.org/CI/Patchwork_2822/

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2822/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx