* [PATCH] drm/i915: Don't dump umpteen thousand requests
@ 2018-04-24 1:24 Chris Wilson
2018-04-24 2:07 ` ✗ Fi.CI.BAT: failure for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Chris Wilson @ 2018-04-24 1:24 UTC (permalink / raw)
To: intel-gfx
If we have more than a few, possibly several thousand request in the
queue, don't show the central portion, just the first few and the last
being executed and/or queued. The first few should be enough to help
identify a problem in execution, and most often comparing the first/last
in the queue is enough to identify problems in the scheduling.
We may need some fine tuning to set MAX_REQUESTS_TO_SHOW for common
debug scenarios, but for the moment if we can avoiding spending more
than a few seconds dumping the GPU state that will avoid a nasty
livelock (where hangcheck spends so long dumping the state, it fires
again and starts to dump the state again in parallel, ad infinitum).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_engine_cs.c | 41 +++++++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 66cddd059666..db28f9e3c306 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1307,11 +1307,13 @@ void intel_engine_dump(struct intel_engine_cs *engine,
struct drm_printer *m,
const char *header, ...)
{
+ const int MAX_REQUESTS_TO_SHOW = 8;
struct intel_breadcrumbs * const b = &engine->breadcrumbs;
const struct intel_engine_execlists * const execlists = &engine->execlists;
struct i915_gpu_error * const error = &engine->i915->gpu_error;
- struct i915_request *rq;
+ struct i915_request *rq, *last;
struct rb_node *rb;
+ int count;
if (header) {
va_list ap;
@@ -1378,16 +1380,47 @@ void intel_engine_dump(struct intel_engine_cs *engine,
}
spin_lock_irq(&engine->timeline->lock);
- list_for_each_entry(rq, &engine->timeline->requests, link)
+
+ last = NULL;
+ count = 0;
+ list_for_each_entry(rq, &engine->timeline->requests, link) {
+ if (count++ < MAX_REQUESTS_TO_SHOW - 1)
+ print_request(m, rq, "\t\tE ");
+ else
+ last = rq;
+ }
+ if (last) {
+ if (count > MAX_REQUESTS_TO_SHOW) {
+ drm_printf(m,
+ "\t\t...skipping %d executing requests...\n",
+ count - MAX_REQUESTS_TO_SHOW);
+ }
print_request(m, rq, "\t\tE ");
+ }
+
+ last = NULL;
+ count = 0;
drm_printf(m, "\t\tQueue priority: %d\n", execlists->queue_priority);
for (rb = execlists->first; rb; rb = rb_next(rb)) {
struct i915_priolist *p =
rb_entry(rb, typeof(*p), node);
- list_for_each_entry(rq, &p->requests, sched.link)
- print_request(m, rq, "\t\tQ ");
+ list_for_each_entry(rq, &p->requests, sched.link) {
+ if (count++ < MAX_REQUESTS_TO_SHOW - 1)
+ print_request(m, rq, "\t\tQ ");
+ else
+ last = rq;
+ }
}
+ if (last) {
+ if (count > MAX_REQUESTS_TO_SHOW) {
+ drm_printf(m,
+ "\t\t...skipping %d queued requests...\n",
+ count - MAX_REQUESTS_TO_SHOW);
+ }
+ print_request(m, last, "\t\tQ ");
+ }
+
spin_unlock_irq(&engine->timeline->lock);
spin_lock_irq(&b->rb_lock);
--
2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Don't dump umpteen thousand requests
2018-04-24 1:24 [PATCH] drm/i915: Don't dump umpteen thousand requests Chris Wilson
@ 2018-04-24 2:07 ` Patchwork
2018-04-24 8:16 ` [PATCH v2] " Chris Wilson
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-04-24 2:07 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Don't dump umpteen thousand requests
URL : https://patchwork.freedesktop.org/series/42151/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_4082 -> Patchwork_8782 =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_8782 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8782, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42151/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_8782:
=== IGT changes ===
==== Possible regressions ====
igt@gem_ringfill@basic-default-hang:
fi-cfl-u: PASS -> INCOMPLETE
fi-cfl-s3: PASS -> INCOMPLETE
fi-skl-6700k2: PASS -> INCOMPLETE
fi-ivb-3770: PASS -> INCOMPLETE
fi-skl-6770hq: PASS -> INCOMPLETE
fi-ivb-3520m: PASS -> INCOMPLETE
fi-ilk-650: PASS -> INCOMPLETE
{fi-kbl-7560u}: PASS -> INCOMPLETE
fi-hsw-4770: PASS -> INCOMPLETE
fi-bsw-n3050: PASS -> INCOMPLETE
fi-skl-6600u: PASS -> INCOMPLETE
fi-cfl-8700k: PASS -> INCOMPLETE
fi-kbl-r: PASS -> INCOMPLETE
fi-kbl-7567u: PASS -> INCOMPLETE
fi-bdw-5557u: PASS -> INCOMPLETE
fi-kbl-7500u: PASS -> INCOMPLETE
fi-snb-2600: PASS -> INCOMPLETE
fi-skl-6260u: PASS -> INCOMPLETE
==== Warnings ====
igt@gem_ringfill@basic-default-hang:
fi-blb-e6850: DMESG-WARN (fdo#101600) -> INCOMPLETE
== Known issues ==
Here are the changes found in Patchwork_8782 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_suspend@basic-s4-devices:
fi-kbl-7500u: PASS -> DMESG-WARN (fdo#105128)
igt@gem_ringfill@basic-default-hang:
fi-skl-gvtdvm: PASS -> INCOMPLETE (fdo#105600)
fi-byt-n2820: PASS -> INCOMPLETE (fdo#102657)
fi-cnl-psr: PASS -> INCOMPLETE (fdo#105086)
fi-cnl-y3: PASS -> INCOMPLETE (fdo#105086)
fi-elk-e7500: PASS -> INCOMPLETE (fdo#103989)
fi-byt-j1900: PASS -> INCOMPLETE (fdo#102657)
fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713)
fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927)
fi-glk-j4005: PASS -> INCOMPLETE (fdo#103359, k.org#198133)
fi-bdw-gvtdvm: PASS -> INCOMPLETE (fdo#105600)
fi-bxt-j4205: PASS -> INCOMPLETE (fdo#103927)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
fdo#102657 https://bugs.freedesktop.org/show_bug.cgi?id=102657
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086
fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
fdo#105600 https://bugs.freedesktop.org/show_bug.cgi?id=105600
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (36 -> 33) ==
Missing (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq
== Build changes ==
* Linux: CI_DRM_4082 -> Patchwork_8782
CI_DRM_4082: 0600266ef9b8407e0dc281acb6b754eba8178c91 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4444: dcc44347494231feabc588c2a76998cbc9afdf8c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8782: fb469863454286bdd6ba86a2b08a373b5340f572 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4444: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit
== Linux commits ==
fb4698634542 drm/i915: Don't dump umpteen thousand requests
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8782/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] drm/i915: Don't dump umpteen thousand requests
2018-04-24 1:24 [PATCH] drm/i915: Don't dump umpteen thousand requests Chris Wilson
2018-04-24 2:07 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2018-04-24 8:16 ` Chris Wilson
2018-04-24 9:27 ` Tvrtko Ursulin
2018-04-24 8:51 ` ✓ Fi.CI.BAT: success for drm/i915: Don't dump umpteen thousand requests (rev2) Patchwork
2018-04-24 9:36 ` ✓ Fi.CI.IGT: " Patchwork
3 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2018-04-24 8:16 UTC (permalink / raw)
To: intel-gfx
If we have more than a few, possibly several thousand request in the
queue, don't show the central portion, just the first few and the last
being executed and/or queued. The first few should be enough to help
identify a problem in execution, and most often comparing the first/last
in the queue is enough to identify problems in the scheduling.
We may need some fine tuning to set MAX_REQUESTS_TO_SHOW for common
debug scenarios, but for the moment if we can avoiding spending more
than a few seconds dumping the GPU state that will avoid a nasty
livelock (where hangcheck spends so long dumping the state, it fires
again and starts to dump the state again in parallel, ad infinitum).
v2: Remember to print last not the stale rq iter after the loop.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_engine_cs.c | 43 +++++++++++++++++++++++---
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 66cddd059666..2398ea71e747 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1307,11 +1307,13 @@ void intel_engine_dump(struct intel_engine_cs *engine,
struct drm_printer *m,
const char *header, ...)
{
+ const int MAX_REQUESTS_TO_SHOW = 8;
struct intel_breadcrumbs * const b = &engine->breadcrumbs;
const struct intel_engine_execlists * const execlists = &engine->execlists;
struct i915_gpu_error * const error = &engine->i915->gpu_error;
- struct i915_request *rq;
+ struct i915_request *rq, *last;
struct rb_node *rb;
+ int count;
if (header) {
va_list ap;
@@ -1378,16 +1380,47 @@ void intel_engine_dump(struct intel_engine_cs *engine,
}
spin_lock_irq(&engine->timeline->lock);
- list_for_each_entry(rq, &engine->timeline->requests, link)
- print_request(m, rq, "\t\tE ");
+
+ last = NULL;
+ count = 0;
+ list_for_each_entry(rq, &engine->timeline->requests, link) {
+ if (count++ < MAX_REQUESTS_TO_SHOW - 1)
+ print_request(m, rq, "\t\tE ");
+ else
+ last = rq;
+ }
+ if (last) {
+ if (count > MAX_REQUESTS_TO_SHOW) {
+ drm_printf(m,
+ "\t\t...skipping %d executing requests...\n",
+ count - MAX_REQUESTS_TO_SHOW);
+ }
+ print_request(m, last, "\t\tE ");
+ }
+
+ last = NULL;
+ count = 0;
drm_printf(m, "\t\tQueue priority: %d\n", execlists->queue_priority);
for (rb = execlists->first; rb; rb = rb_next(rb)) {
struct i915_priolist *p =
rb_entry(rb, typeof(*p), node);
- list_for_each_entry(rq, &p->requests, sched.link)
- print_request(m, rq, "\t\tQ ");
+ list_for_each_entry(rq, &p->requests, sched.link) {
+ if (count++ < MAX_REQUESTS_TO_SHOW - 1)
+ print_request(m, rq, "\t\tQ ");
+ else
+ last = rq;
+ }
}
+ if (last) {
+ if (count > MAX_REQUESTS_TO_SHOW) {
+ drm_printf(m,
+ "\t\t...skipping %d queued requests...\n",
+ count - MAX_REQUESTS_TO_SHOW);
+ }
+ print_request(m, last, "\t\tQ ");
+ }
+
spin_unlock_irq(&engine->timeline->lock);
spin_lock_irq(&b->rb_lock);
--
2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Don't dump umpteen thousand requests (rev2)
2018-04-24 1:24 [PATCH] drm/i915: Don't dump umpteen thousand requests Chris Wilson
2018-04-24 2:07 ` ✗ Fi.CI.BAT: failure for " Patchwork
2018-04-24 8:16 ` [PATCH v2] " Chris Wilson
@ 2018-04-24 8:51 ` Patchwork
2018-04-24 9:36 ` ✓ Fi.CI.IGT: " Patchwork
3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-04-24 8:51 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Don't dump umpteen thousand requests (rev2)
URL : https://patchwork.freedesktop.org/series/42151/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4083 -> Patchwork_8784 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42151/revisions/2/mbox/
== Known issues ==
Here are the changes found in Patchwork_8784 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-cnl-y3: PASS -> DMESG-WARN (fdo#104951)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-cnl-psr: PASS -> DMESG-WARN (fdo#104951)
fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
== Participating hosts (36 -> 33) ==
Missing (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq
== Build changes ==
* Linux: CI_DRM_4083 -> Patchwork_8784
CI_DRM_4083: b4e886c1f0e23ba0506206ea6758a60c05bd64c8 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4444: dcc44347494231feabc588c2a76998cbc9afdf8c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8784: 97f32277188caa7e317929991a22b3570d0078d9 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4444: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit
== Linux commits ==
97f32277188c drm/i915: Don't dump umpteen thousand requests
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8784/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/i915: Don't dump umpteen thousand requests
2018-04-24 8:16 ` [PATCH v2] " Chris Wilson
@ 2018-04-24 9:27 ` Tvrtko Ursulin
2018-04-24 9:33 ` Chris Wilson
0 siblings, 1 reply; 9+ messages in thread
From: Tvrtko Ursulin @ 2018-04-24 9:27 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 24/04/2018 09:16, Chris Wilson wrote:
> If we have more than a few, possibly several thousand request in the
> queue, don't show the central portion, just the first few and the last
> being executed and/or queued. The first few should be enough to help
> identify a problem in execution, and most often comparing the first/last
> in the queue is enough to identify problems in the scheduling.
>
> We may need some fine tuning to set MAX_REQUESTS_TO_SHOW for common
> debug scenarios, but for the moment if we can avoiding spending more
> than a few seconds dumping the GPU state that will avoid a nasty
> livelock (where hangcheck spends so long dumping the state, it fires
> again and starts to dump the state again in parallel, ad infinitum).
>
> v2: Remember to print last not the stale rq iter after the loop.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_engine_cs.c | 43 +++++++++++++++++++++++---
> 1 file changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 66cddd059666..2398ea71e747 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1307,11 +1307,13 @@ void intel_engine_dump(struct intel_engine_cs *engine,
> struct drm_printer *m,
> const char *header, ...)
> {
> + const int MAX_REQUESTS_TO_SHOW = 8;
> struct intel_breadcrumbs * const b = &engine->breadcrumbs;
> const struct intel_engine_execlists * const execlists = &engine->execlists;
> struct i915_gpu_error * const error = &engine->i915->gpu_error;
> - struct i915_request *rq;
> + struct i915_request *rq, *last;
> struct rb_node *rb;
> + int count;
>
> if (header) {
> va_list ap;
> @@ -1378,16 +1380,47 @@ void intel_engine_dump(struct intel_engine_cs *engine,
> }
>
> spin_lock_irq(&engine->timeline->lock);
> - list_for_each_entry(rq, &engine->timeline->requests, link)
> - print_request(m, rq, "\t\tE ");
> +
> + last = NULL;
> + count = 0;
> + list_for_each_entry(rq, &engine->timeline->requests, link) {
> + if (count++ < MAX_REQUESTS_TO_SHOW - 1)
> + print_request(m, rq, "\t\tE ");
> + else
> + last = rq;
else {
last = list_last_entry(...) ?
break;
}
> + }
> + if (last) {
> + if (count > MAX_REQUESTS_TO_SHOW) {
> + drm_printf(m,
> + "\t\t...skipping %d executing requests...\n",
> + count - MAX_REQUESTS_TO_SHOW);
> + }
> + print_request(m, last, "\t\tE ");
> + }
Or even stuff this printf in the first loop above, under the else
branch. Maybe shorter would be like this, module off by ones if I made some:
list_for_each_entry(rq, &engine->timeline->requests, link) {
struct i915_request *pr = rq;
if (++count > MAX_REQUESTS_TO_SHOW) {
pr = list_last_entry(...);
drm_printf(m,
"\t\t...skipping %d executing requests...\n",
count - MAX_REQUESTS_TO_SHOW);
}
print_request(m, pr, "\t\tE ");
if (count > MAX_REQUESTS_TO_SHOW)
break;
}
> +
> + last = NULL;
> + count = 0;
> drm_printf(m, "\t\tQueue priority: %d\n", execlists->queue_priority);
> for (rb = execlists->first; rb; rb = rb_next(rb)) {
> struct i915_priolist *p =
> rb_entry(rb, typeof(*p), node);
>
> - list_for_each_entry(rq, &p->requests, sched.link)
> - print_request(m, rq, "\t\tQ ");
> + list_for_each_entry(rq, &p->requests, sched.link) {
> + if (count++ < MAX_REQUESTS_TO_SHOW - 1)
> + print_request(m, rq, "\t\tQ ");
> + else
> + last = rq;
> + }
> }
> + if (last) {
> + if (count > MAX_REQUESTS_TO_SHOW) {
> + drm_printf(m,
> + "\t\t...skipping %d queued requests...\n",
> + count - MAX_REQUESTS_TO_SHOW);
> + }
> + print_request(m, last, "\t\tQ ");
> + }
Then I am thinking how to avoid the duplication and extract the smart
printer. Macro would be easy at least, if a bit ugly.
Another idea comes to mind, but probably for the future, to merge same
prio, context and strictly consecutive seqnos to a single line of output
like:
[prefix]seqno-seqno [%llx:seqno-seqno] (%u consecutive requests)
Give or take, but it will be more involved to do that.
> +
> spin_unlock_irq(&engine->timeline->lock);
>
> spin_lock_irq(&b->rb_lock);
>
Looks OK in general, just please see if you like my idea to contain the
logic within a single loop and maybe even move it to a macro.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/i915: Don't dump umpteen thousand requests
2018-04-24 9:27 ` Tvrtko Ursulin
@ 2018-04-24 9:33 ` Chris Wilson
2018-04-24 9:40 ` Tvrtko Ursulin
0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2018-04-24 9:33 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2018-04-24 10:27:23)
>
> On 24/04/2018 09:16, Chris Wilson wrote:
> > If we have more than a few, possibly several thousand request in the
> > queue, don't show the central portion, just the first few and the last
> > being executed and/or queued. The first few should be enough to help
> > identify a problem in execution, and most often comparing the first/last
> > in the queue is enough to identify problems in the scheduling.
> >
> > We may need some fine tuning to set MAX_REQUESTS_TO_SHOW for common
> > debug scenarios, but for the moment if we can avoiding spending more
> > than a few seconds dumping the GPU state that will avoid a nasty
> > livelock (where hangcheck spends so long dumping the state, it fires
> > again and starts to dump the state again in parallel, ad infinitum).
> >
> > v2: Remember to print last not the stale rq iter after the loop.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/intel_engine_cs.c | 43 +++++++++++++++++++++++---
> > 1 file changed, 38 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index 66cddd059666..2398ea71e747 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -1307,11 +1307,13 @@ void intel_engine_dump(struct intel_engine_cs *engine,
> > struct drm_printer *m,
> > const char *header, ...)
> > {
> > + const int MAX_REQUESTS_TO_SHOW = 8;
> > struct intel_breadcrumbs * const b = &engine->breadcrumbs;
> > const struct intel_engine_execlists * const execlists = &engine->execlists;
> > struct i915_gpu_error * const error = &engine->i915->gpu_error;
> > - struct i915_request *rq;
> > + struct i915_request *rq, *last;
> > struct rb_node *rb;
> > + int count;
> >
> > if (header) {
> > va_list ap;
> > @@ -1378,16 +1380,47 @@ void intel_engine_dump(struct intel_engine_cs *engine,
> > }
> >
> > spin_lock_irq(&engine->timeline->lock);
> > - list_for_each_entry(rq, &engine->timeline->requests, link)
> > - print_request(m, rq, " E ");
> > +
> > + last = NULL;
> > + count = 0;
> > + list_for_each_entry(rq, &engine->timeline->requests, link) {
> > + if (count++ < MAX_REQUESTS_TO_SHOW - 1)
> > + print_request(m, rq, " E ");
> > + else
> > + last = rq;
>
> else {
> last = list_last_entry(...) ?
> break;
> }
>
> > + }
> > + if (last) {
> > + if (count > MAX_REQUESTS_TO_SHOW) {
> > + drm_printf(m,
> > + " ...skipping %d executing requests...\n",
> > + count - MAX_REQUESTS_TO_SHOW);
> > + }
> > + print_request(m, last, " E ");
> > + }
>
> Or even stuff this printf in the first loop above, under the else
> branch. Maybe shorter would be like this, module off by ones if I made some:
>
> list_for_each_entry(rq, &engine->timeline->requests, link) {
> struct i915_request *pr = rq;
>
> if (++count > MAX_REQUESTS_TO_SHOW) {
> pr = list_last_entry(...);
> drm_printf(m,
> " ...skipping %d executing requests...\n",
> count - MAX_REQUESTS_TO_SHOW);
> }
>
> print_request(m, pr, " E ");
>
> if (count > MAX_REQUESTS_TO_SHOW)
> break;
> }
>
> > +
> > + last = NULL;
> > + count = 0;
> > drm_printf(m, " Queue priority: %d\n", execlists->queue_priority);
> > for (rb = execlists->first; rb; rb = rb_next(rb)) {
> > struct i915_priolist *p =
> > rb_entry(rb, typeof(*p), node);
> >
> > - list_for_each_entry(rq, &p->requests, sched.link)
> > - print_request(m, rq, " Q ");
> > + list_for_each_entry(rq, &p->requests, sched.link) {
> > + if (count++ < MAX_REQUESTS_TO_SHOW - 1)
> > + print_request(m, rq, " Q ");
> > + else
> > + last = rq;
> > + }
> > }
> > + if (last) {
> > + if (count > MAX_REQUESTS_TO_SHOW) {
> > + drm_printf(m,
> > + " ...skipping %d queued requests...\n",
> > + count - MAX_REQUESTS_TO_SHOW);
> > + }
> > + print_request(m, last, " Q ");
> > + }
>
> Then I am thinking how to avoid the duplication and extract the smart
> printer. Macro would be easy at least, if a bit ugly.
Yeah, and for the moment, I'd like to keep the duplication obvious and
keep the two passes very similar.
> Another idea comes to mind, but probably for the future, to merge same
> prio, context and strictly consecutive seqnos to a single line of output
> like:
>
> [prefix]seqno-seqno [%llx:seqno-seqno] (%u consecutive requests)
>
> Give or take, but it will be more involved to do that.
Deciding when rq are equivalent will get messy, and going to drift.
>
> > +
> > spin_unlock_irq(&engine->timeline->lock);
> >
> > spin_lock_irq(&b->rb_lock);
> >
>
> Looks OK in general, just please see if you like my idea to contain the
> logic within a single loop and maybe even move it to a macro.
I don't, because it missed one important thing, the count of skipped
requests. :-p
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Don't dump umpteen thousand requests (rev2)
2018-04-24 1:24 [PATCH] drm/i915: Don't dump umpteen thousand requests Chris Wilson
` (2 preceding siblings ...)
2018-04-24 8:51 ` ✓ Fi.CI.BAT: success for drm/i915: Don't dump umpteen thousand requests (rev2) Patchwork
@ 2018-04-24 9:36 ` Patchwork
3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-04-24 9:36 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Don't dump umpteen thousand requests (rev2)
URL : https://patchwork.freedesktop.org/series/42151/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4083_full -> Patchwork_8784_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_8784_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8784_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42151/revisions/2/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_8784_full:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_schedule@deep-bsd2:
shard-kbl: SKIP -> PASS
igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
shard-snb: SKIP -> PASS +2
igt@kms_universal_plane@universal-plane-pipe-b-sanity:
shard-glk: PASS -> SKIP +87
== Known issues ==
Here are the changes found in Patchwork_8784_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_flip@basic-flip-vs-wf_vblank:
shard-hsw: PASS -> FAIL (fdo#103928)
igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible:
shard-apl: PASS -> FAIL (fdo#100368)
igt@kms_hdmi_inject@inject-audio:
shard-glk: PASS -> FAIL (fdo#102370)
igt@kms_sysfs_edid_timing:
shard-apl: PASS -> WARN (fdo#100047)
==== Possible fixes ====
igt@drv_selftest@live_gtt:
shard-apl: INCOMPLETE (fdo#103927) -> PASS
igt@kms_flip@flip-vs-wf_vblank-interruptible:
shard-glk: FAIL (fdo#100368) -> PASS +1
igt@kms_setmode@basic:
shard-glk: FAIL (fdo#99912) -> PASS
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102370 https://bugs.freedesktop.org/show_bug.cgi?id=102370
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (6 -> 5) ==
Missing (1): shard-glkb
== Build changes ==
* Linux: CI_DRM_4083 -> Patchwork_8784
CI_DRM_4083: b4e886c1f0e23ba0506206ea6758a60c05bd64c8 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4444: dcc44347494231feabc588c2a76998cbc9afdf8c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8784: 97f32277188caa7e317929991a22b3570d0078d9 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4444: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8784/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/i915: Don't dump umpteen thousand requests
2018-04-24 9:33 ` Chris Wilson
@ 2018-04-24 9:40 ` Tvrtko Ursulin
2018-04-24 13:02 ` Chris Wilson
0 siblings, 1 reply; 9+ messages in thread
From: Tvrtko Ursulin @ 2018-04-24 9:40 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 24/04/2018 10:33, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-04-24 10:27:23)
>>
>> On 24/04/2018 09:16, Chris Wilson wrote:
>>> If we have more than a few, possibly several thousand request in the
>>> queue, don't show the central portion, just the first few and the last
>>> being executed and/or queued. The first few should be enough to help
>>> identify a problem in execution, and most often comparing the first/last
>>> in the queue is enough to identify problems in the scheduling.
>>>
>>> We may need some fine tuning to set MAX_REQUESTS_TO_SHOW for common
>>> debug scenarios, but for the moment if we can avoiding spending more
>>> than a few seconds dumping the GPU state that will avoid a nasty
>>> livelock (where hangcheck spends so long dumping the state, it fires
>>> again and starts to dump the state again in parallel, ad infinitum).
>>>
>>> v2: Remember to print last not the stale rq iter after the loop.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>> drivers/gpu/drm/i915/intel_engine_cs.c | 43 +++++++++++++++++++++++---
>>> 1 file changed, 38 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
>>> index 66cddd059666..2398ea71e747 100644
>>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
>>> @@ -1307,11 +1307,13 @@ void intel_engine_dump(struct intel_engine_cs *engine,
>>> struct drm_printer *m,
>>> const char *header, ...)
>>> {
>>> + const int MAX_REQUESTS_TO_SHOW = 8;
>>> struct intel_breadcrumbs * const b = &engine->breadcrumbs;
>>> const struct intel_engine_execlists * const execlists = &engine->execlists;
>>> struct i915_gpu_error * const error = &engine->i915->gpu_error;
>>> - struct i915_request *rq;
>>> + struct i915_request *rq, *last;
>>> struct rb_node *rb;
>>> + int count;
>>>
>>> if (header) {
>>> va_list ap;
>>> @@ -1378,16 +1380,47 @@ void intel_engine_dump(struct intel_engine_cs *engine,
>>> }
>>>
>>> spin_lock_irq(&engine->timeline->lock);
>>> - list_for_each_entry(rq, &engine->timeline->requests, link)
>>> - print_request(m, rq, " E ");
>>> +
>>> + last = NULL;
>>> + count = 0;
>>> + list_for_each_entry(rq, &engine->timeline->requests, link) {
>>> + if (count++ < MAX_REQUESTS_TO_SHOW - 1)
>>> + print_request(m, rq, " E ");
>>> + else
>>> + last = rq;
>>
>> else {
>> last = list_last_entry(...) ?
>> break;
>> }
>>
>>> + }
>>> + if (last) {
>>> + if (count > MAX_REQUESTS_TO_SHOW) {
>>> + drm_printf(m,
>>> + " ...skipping %d executing requests...\n",
>>> + count - MAX_REQUESTS_TO_SHOW);
>>> + }
>>> + print_request(m, last, " E ");
>>> + }
>>
>> Or even stuff this printf in the first loop above, under the else
>> branch. Maybe shorter would be like this, module off by ones if I made some:
>>
>> list_for_each_entry(rq, &engine->timeline->requests, link) {
>> struct i915_request *pr = rq;
>>
>> if (++count > MAX_REQUESTS_TO_SHOW) {
>> pr = list_last_entry(...);
>> drm_printf(m,
>> " ...skipping %d executing requests...\n",
>> count - MAX_REQUESTS_TO_SHOW);
>> }
>>
>> print_request(m, pr, " E ");
>>
>> if (count > MAX_REQUESTS_TO_SHOW)
>> break;
>> }
>>
>>> +
>>> + last = NULL;
>>> + count = 0;
>>> drm_printf(m, " Queue priority: %d\n", execlists->queue_priority);
>>> for (rb = execlists->first; rb; rb = rb_next(rb)) {
>>> struct i915_priolist *p =
>>> rb_entry(rb, typeof(*p), node);
>>>
>>> - list_for_each_entry(rq, &p->requests, sched.link)
>>> - print_request(m, rq, " Q ");
>>> + list_for_each_entry(rq, &p->requests, sched.link) {
>>> + if (count++ < MAX_REQUESTS_TO_SHOW - 1)
>>> + print_request(m, rq, " Q ");
>>> + else
>>> + last = rq;
>>> + }
>>> }
>>> + if (last) {
>>> + if (count > MAX_REQUESTS_TO_SHOW) {
>>> + drm_printf(m,
>>> + " ...skipping %d queued requests...\n",
>>> + count - MAX_REQUESTS_TO_SHOW);
>>> + }
>>> + print_request(m, last, " Q ");
>>> + }
>>
>> Then I am thinking how to avoid the duplication and extract the smart
>> printer. Macro would be easy at least, if a bit ugly.
>
> Yeah, and for the moment, I'd like to keep the duplication obvious and
> keep the two passes very similar.
>
>> Another idea comes to mind, but probably for the future, to merge same
>> prio, context and strictly consecutive seqnos to a single line of output
>> like:
>>
>> [prefix]seqno-seqno [%llx:seqno-seqno] (%u consecutive requests)
>>
>> Give or take, but it will be more involved to do that.
>
> Deciding when rq are equivalent will get messy, and going to drift.
>
>>
>>> +
>>> spin_unlock_irq(&engine->timeline->lock);
>>>
>>> spin_lock_irq(&b->rb_lock);
>>>
>>
>> Looks OK in general, just please see if you like my idea to contain the
>> logic within a single loop and maybe even move it to a macro.
>
> I don't, because it missed one important thing, the count of skipped
> requests. :-p
Doh.. Well, I don't like it, but have no better/easier ideas at the moment.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/i915: Don't dump umpteen thousand requests
2018-04-24 9:40 ` Tvrtko Ursulin
@ 2018-04-24 13:02 ` Chris Wilson
0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2018-04-24 13:02 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2018-04-24 10:40:57)
>
> On 24/04/2018 10:33, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-04-24 10:27:23)
> >>
> >> On 24/04/2018 09:16, Chris Wilson wrote:
> >>> If we have more than a few, possibly several thousand request in the
> >>> queue, don't show the central portion, just the first few and the last
> >>> being executed and/or queued. The first few should be enough to help
> >>> identify a problem in execution, and most often comparing the first/last
> >>> in the queue is enough to identify problems in the scheduling.
> >>>
> >>> We may need some fine tuning to set MAX_REQUESTS_TO_SHOW for common
> >>> debug scenarios, but for the moment if we can avoiding spending more
> >>> than a few seconds dumping the GPU state that will avoid a nasty
> >>> livelock (where hangcheck spends so long dumping the state, it fires
> >>> again and starts to dump the state again in parallel, ad infinitum).
> >>>
> >>> v2: Remember to print last not the stale rq iter after the loop.
> >>>
> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> ---
> >>> drivers/gpu/drm/i915/intel_engine_cs.c | 43 +++++++++++++++++++++++---
> >>> 1 file changed, 38 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> >>> index 66cddd059666..2398ea71e747 100644
> >>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> >>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> >>> @@ -1307,11 +1307,13 @@ void intel_engine_dump(struct intel_engine_cs *engine,
> >>> struct drm_printer *m,
> >>> const char *header, ...)
> >>> {
> >>> + const int MAX_REQUESTS_TO_SHOW = 8;
> >>> struct intel_breadcrumbs * const b = &engine->breadcrumbs;
> >>> const struct intel_engine_execlists * const execlists = &engine->execlists;
> >>> struct i915_gpu_error * const error = &engine->i915->gpu_error;
> >>> - struct i915_request *rq;
> >>> + struct i915_request *rq, *last;
> >>> struct rb_node *rb;
> >>> + int count;
> >>>
> >>> if (header) {
> >>> va_list ap;
> >>> @@ -1378,16 +1380,47 @@ void intel_engine_dump(struct intel_engine_cs *engine,
> >>> }
> >>>
> >>> spin_lock_irq(&engine->timeline->lock);
> >>> - list_for_each_entry(rq, &engine->timeline->requests, link)
> >>> - print_request(m, rq, " E ");
> >>> +
> >>> + last = NULL;
> >>> + count = 0;
> >>> + list_for_each_entry(rq, &engine->timeline->requests, link) {
> >>> + if (count++ < MAX_REQUESTS_TO_SHOW - 1)
> >>> + print_request(m, rq, " E ");
> >>> + else
> >>> + last = rq;
> >>
> >> else {
> >> last = list_last_entry(...) ?
> >> break;
> >> }
> >>
> >>> + }
> >>> + if (last) {
> >>> + if (count > MAX_REQUESTS_TO_SHOW) {
> >>> + drm_printf(m,
> >>> + " ...skipping %d executing requests...\n",
> >>> + count - MAX_REQUESTS_TO_SHOW);
> >>> + }
> >>> + print_request(m, last, " E ");
> >>> + }
> >>
> >> Or even stuff this printf in the first loop above, under the else
> >> branch. Maybe shorter would be like this, module off by ones if I made some:
> >>
> >> list_for_each_entry(rq, &engine->timeline->requests, link) {
> >> struct i915_request *pr = rq;
> >>
> >> if (++count > MAX_REQUESTS_TO_SHOW) {
> >> pr = list_last_entry(...);
> >> drm_printf(m,
> >> " ...skipping %d executing requests...\n",
> >> count - MAX_REQUESTS_TO_SHOW);
> >> }
> >>
> >> print_request(m, pr, " E ");
> >>
> >> if (count > MAX_REQUESTS_TO_SHOW)
> >> break;
> >> }
> >>
> >>> +
> >>> + last = NULL;
> >>> + count = 0;
> >>> drm_printf(m, " Queue priority: %d\n", execlists->queue_priority);
> >>> for (rb = execlists->first; rb; rb = rb_next(rb)) {
> >>> struct i915_priolist *p =
> >>> rb_entry(rb, typeof(*p), node);
> >>>
> >>> - list_for_each_entry(rq, &p->requests, sched.link)
> >>> - print_request(m, rq, " Q ");
> >>> + list_for_each_entry(rq, &p->requests, sched.link) {
> >>> + if (count++ < MAX_REQUESTS_TO_SHOW - 1)
> >>> + print_request(m, rq, " Q ");
> >>> + else
> >>> + last = rq;
> >>> + }
> >>> }
> >>> + if (last) {
> >>> + if (count > MAX_REQUESTS_TO_SHOW) {
> >>> + drm_printf(m,
> >>> + " ...skipping %d queued requests...\n",
> >>> + count - MAX_REQUESTS_TO_SHOW);
> >>> + }
> >>> + print_request(m, last, " Q ");
> >>> + }
> >>
> >> Then I am thinking how to avoid the duplication and extract the smart
> >> printer. Macro would be easy at least, if a bit ugly.
> >
> > Yeah, and for the moment, I'd like to keep the duplication obvious and
> > keep the two passes very similar.
> >
> >> Another idea comes to mind, but probably for the future, to merge same
> >> prio, context and strictly consecutive seqnos to a single line of output
> >> like:
> >>
> >> [prefix]seqno-seqno [%llx:seqno-seqno] (%u consecutive requests)
> >>
> >> Give or take, but it will be more involved to do that.
> >
> > Deciding when rq are equivalent will get messy, and going to drift.
> >
> >>
> >>> +
> >>> spin_unlock_irq(&engine->timeline->lock);
> >>>
> >>> spin_lock_irq(&b->rb_lock);
> >>>
> >>
> >> Looks OK in general, just please see if you like my idea to contain the
> >> logic within a single loop and maybe even move it to a macro.
> >
> > I don't, because it missed one important thing, the count of skipped
> > requests. :-p
>
> Doh.. Well, I don't like it, but have no better/easier ideas at the moment.
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
I'll take it as an improvement that can be refined again and again and
again...
Thanks,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-04-24 13:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-24 1:24 [PATCH] drm/i915: Don't dump umpteen thousand requests Chris Wilson
2018-04-24 2:07 ` ✗ Fi.CI.BAT: failure for " Patchwork
2018-04-24 8:16 ` [PATCH v2] " Chris Wilson
2018-04-24 9:27 ` Tvrtko Ursulin
2018-04-24 9:33 ` Chris Wilson
2018-04-24 9:40 ` Tvrtko Ursulin
2018-04-24 13:02 ` Chris Wilson
2018-04-24 8:51 ` ✓ Fi.CI.BAT: success for drm/i915: Don't dump umpteen thousand requests (rev2) Patchwork
2018-04-24 9:36 ` ✓ Fi.CI.IGT: " Patchwork
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.