* [PATCH] drm/i915: Remove function details from device error messages
@ 2018-07-09 13:48 Chris Wilson
2018-07-09 14:36 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Chris Wilson @ 2018-07-09 13:48 UTC (permalink / raw)
To: intel-gfx
Error messages are intended to be addressed to the user; be clear,
succinct, instructive and unambiguous. Adding the function name to
that message does not add any information the user requires and in
the process makes the message less clear.
E.g.
[ 245.539711] i915 0000:00:02.0: [drm:i915_gem_init [i915]] Failed to initialize GPU, declaring it wedged!
becomes
[ 245.539711] i915 0000:00:02.0: Failed to initialize GPU, declaring it wedged!
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_drv.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2959c88a37a5..c2b9a4a0ee49 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -104,8 +104,13 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
vaf.fmt = fmt;
vaf.va = &args;
- dev_printk(level, kdev, "[" DRM_NAME ":%ps] %pV",
- __builtin_return_address(0), &vaf);
+ if (is_error)
+ dev_printk(level, kdev, "%pV", &vaf);
+ else
+ dev_printk(level, kdev, "[" DRM_NAME ":%ps] %pV",
+ __builtin_return_address(0), &vaf);
+
+ va_end(args);
if (is_error && !shown_bug_once) {
/*
@@ -117,8 +122,6 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
dev_notice(kdev, "%s", FDO_BUG_MSG);
shown_bug_once = true;
}
-
- va_end(args);
}
/* Map PCH device id to PCH type, or PCH_NONE if unknown. */
--
2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* ✓ Fi.CI.BAT: success for drm/i915: Remove function details from device error messages
2018-07-09 13:48 [PATCH] drm/i915: Remove function details from device error messages Chris Wilson
@ 2018-07-09 14:36 ` Patchwork
2018-07-09 17:51 ` [PATCH] " Rodrigo Vivi
2018-07-09 20:57 ` ✗ Fi.CI.IGT: failure for " Patchwork
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-07-09 14:36 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Remove function details from device error messages
URL : https://patchwork.freedesktop.org/series/46187/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4454 -> Patchwork_9594 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/46187/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9594 that come from known issues:
=== IGT changes ===
==== Possible fixes ====
igt@gem_exec_suspend@basic-s3:
{fi-kbl-8809g}: FAIL (fdo#103375) -> PASS
==== Warnings ====
igt@drv_module_reload@basic-no-display:
{fi-skl-iommu}: DMESG-FAIL -> FAIL (fdo#106066) +2
igt@gem_exec_suspend@basic-s4-devices:
{fi-kbl-8809g}: FAIL -> INCOMPLETE (fdo#107139)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#106066 https://bugs.freedesktop.org/show_bug.cgi?id=106066
fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
== Participating hosts (46 -> 42) ==
Additional (1): fi-cfl-8109u
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4454 -> Patchwork_9594
CI_DRM_4454: 5f4ec795dbe0b8a1c565afcd2af79e41346e7268 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4544: 764160f214cd916ddb79408b9f28ac0ad2df40e0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9594: 895b666733cb050558dbe5826be430b17fb694b3 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
895b666733cb drm/i915: Remove function details from device error messages
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9594/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Remove function details from device error messages
2018-07-09 13:48 [PATCH] drm/i915: Remove function details from device error messages Chris Wilson
2018-07-09 14:36 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-07-09 17:51 ` Rodrigo Vivi
2018-07-09 20:14 ` Chris Wilson
2018-07-09 20:57 ` ✗ Fi.CI.IGT: failure for " Patchwork
2 siblings, 1 reply; 6+ messages in thread
From: Rodrigo Vivi @ 2018-07-09 17:51 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Mon, Jul 09, 2018 at 02:48:58PM +0100, Chris Wilson wrote:
> Error messages are intended to be addressed to the user; be clear,
> succinct, instructive and unambiguous. Adding the function name to
> that message does not add any information the user requires and in
> the process makes the message less clear.
>
> E.g.
>
> [ 245.539711] i915 0000:00:02.0: [drm:i915_gem_init [i915]] Failed to initialize GPU, declaring it wedged!
Overall I like the idea...
The down side is that for us when debugging we would need to always trust grep like
searches and many debug messages are constructed out of variables what makes it a bit
hard to find sometimes. Ok, nothing that we couldn't figure out...
>
> becomes
>
> [ 245.539711] i915 0000:00:02.0: Failed to initialize GPU, declaring it wedged!
What about adding an "ERROR:" ?
[ 245.539711] i915 0000:00:02.0: ERROR: Failed to initialize GPU, declaring it wedged!
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2959c88a37a5..c2b9a4a0ee49 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -104,8 +104,13 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
> vaf.fmt = fmt;
> vaf.va = &args;
>
> - dev_printk(level, kdev, "[" DRM_NAME ":%ps] %pV",
> - __builtin_return_address(0), &vaf);
> + if (is_error)
> + dev_printk(level, kdev, "%pV", &vaf);
> + else
> + dev_printk(level, kdev, "[" DRM_NAME ":%ps] %pV",
> + __builtin_return_address(0), &vaf);
> +
> + va_end(args);
>
> if (is_error && !shown_bug_once) {
> /*
> @@ -117,8 +122,6 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
> dev_notice(kdev, "%s", FDO_BUG_MSG);
> shown_bug_once = true;
> }
> -
> - va_end(args);
> }
>
> /* Map PCH device id to PCH type, or PCH_NONE if unknown. */
> --
> 2.18.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Remove function details from device error messages
2018-07-09 17:51 ` [PATCH] " Rodrigo Vivi
@ 2018-07-09 20:14 ` Chris Wilson
2018-07-09 21:13 ` Rodrigo Vivi
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2018-07-09 20:14 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
Quoting Rodrigo Vivi (2018-07-09 18:51:02)
> On Mon, Jul 09, 2018 at 02:48:58PM +0100, Chris Wilson wrote:
> > Error messages are intended to be addressed to the user; be clear,
> > succinct, instructive and unambiguous. Adding the function name to
> > that message does not add any information the user requires and in
> > the process makes the message less clear.
> >
> > E.g.
> >
> > [ 245.539711] i915 0000:00:02.0: [drm:i915_gem_init [i915]] Failed to initialize GPU, declaring it wedged!
>
> Overall I like the idea...
>
> The down side is that for us when debugging we would need to always trust grep like
> searches and many debug messages are constructed out of variables what makes it a bit
> hard to find sometimes. Ok, nothing that we couldn't figure out...
A big difference is that error messages are targeted at the user, and as
such should be succinct and not require them to dig into the source code
to understand what it means and what action they need to take. Usually
such error messages are accompanied by a lot of debug output for
developers to pour over, but for the average user, imo we just need to
say what broke and no longer works, and how they can file a bug (any
information we need for that bug should be captured automatically and
read for them to attach).
> > becomes
> >
> > [ 245.539711] i915 0000:00:02.0: Failed to initialize GPU, declaring it wedged!
>
> What about adding an "ERROR:" ?
The KERN_ERR is recorded in the output for the userspace application to
decide how to colourize and highlight when it presents the kmsg records.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Remove function details from device error messages
2018-07-09 20:14 ` Chris Wilson
@ 2018-07-09 21:13 ` Rodrigo Vivi
0 siblings, 0 replies; 6+ messages in thread
From: Rodrigo Vivi @ 2018-07-09 21:13 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Mon, Jul 09, 2018 at 09:14:05PM +0100, Chris Wilson wrote:
> Quoting Rodrigo Vivi (2018-07-09 18:51:02)
> > On Mon, Jul 09, 2018 at 02:48:58PM +0100, Chris Wilson wrote:
> > > Error messages are intended to be addressed to the user; be clear,
> > > succinct, instructive and unambiguous. Adding the function name to
> > > that message does not add any information the user requires and in
> > > the process makes the message less clear.
> > >
> > > E.g.
> > >
> > > [ 245.539711] i915 0000:00:02.0: [drm:i915_gem_init [i915]] Failed to initialize GPU, declaring it wedged!
> >
> > Overall I like the idea...
> >
> > The down side is that for us when debugging we would need to always trust grep like
> > searches and many debug messages are constructed out of variables what makes it a bit
> > hard to find sometimes. Ok, nothing that we couldn't figure out...
>
> A big difference is that error messages are targeted at the user, and as
> such should be succinct and not require them to dig into the source code
> to understand what it means and what action they need to take. Usually
> such error messages are accompanied by a lot of debug output for
> developers to pour over, but for the average user, imo we just need to
> say what broke and no longer works, and how they can file a bug (any
> information we need for that bug should be captured automatically and
> read for them to attach).
it makes sense.
>
> > > becomes
> > >
> > > [ 245.539711] i915 0000:00:02.0: Failed to initialize GPU, declaring it wedged!
> >
> > What about adding an "ERROR:" ?
>
> The KERN_ERR is recorded in the output for the userspace application to
> decide how to colourize and highlight when it presents the kmsg records.
hmm... good point.
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✗ Fi.CI.IGT: failure for drm/i915: Remove function details from device error messages
2018-07-09 13:48 [PATCH] drm/i915: Remove function details from device error messages Chris Wilson
2018-07-09 14:36 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-07-09 17:51 ` [PATCH] " Rodrigo Vivi
@ 2018-07-09 20:57 ` Patchwork
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-07-09 20:57 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Remove function details from device error messages
URL : https://patchwork.freedesktop.org/series/46187/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_4454_full -> Patchwork_9594_full =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_9594_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9594_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9594_full:
=== IGT changes ===
==== Possible regressions ====
igt@kms_universal_plane@cursor-fb-leak-pipe-c:
shard-apl: PASS -> FAIL
==== Warnings ====
igt@gem_exec_schedule@deep-bsd1:
shard-kbl: PASS -> SKIP +1
== Known issues ==
Here are the changes found in Patchwork_9594_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
shard-glk: PASS -> FAIL (fdo#106509)
igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
shard-hsw: PASS -> FAIL (fdo#103355)
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: PASS -> FAIL (fdo#105363)
igt@kms_flip@plain-flip-ts-check:
shard-glk: PASS -> FAIL (fdo#100368)
igt@kms_flip_tiling@flip-to-x-tiled:
shard-glk: PASS -> FAIL (fdo#107161)
igt@kms_flip_tiling@flip-y-tiled:
shard-glk: PASS -> FAIL (fdo#103822, fdo#107161)
igt@kms_setmode@basic:
shard-apl: PASS -> FAIL (fdo#99912)
igt@pm_rpm@system-suspend:
shard-kbl: PASS -> INCOMPLETE (fdo#103665)
==== Possible fixes ====
igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
shard-glk: DMESG-WARN (fdo#106538, fdo#105763) -> PASS
igt@kms_flip@2x-flip-vs-expired-vblank:
shard-glk: FAIL (fdo#102887) -> PASS
igt@kms_flip_tiling@flip-to-y-tiled:
shard-glk: FAIL (fdo#107161) -> PASS
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-cpu:
shard-hsw: FAIL (fdo#103167, fdo#105682) -> PASS
igt@kms_setmode@basic:
shard-kbl: FAIL (fdo#99912) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#107161 https://bugs.freedesktop.org/show_bug.cgi?id=107161
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4454 -> Patchwork_9594
CI_DRM_4454: 5f4ec795dbe0b8a1c565afcd2af79e41346e7268 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4544: 764160f214cd916ddb79408b9f28ac0ad2df40e0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9594: 895b666733cb050558dbe5826be430b17fb694b3 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9594/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-09 21:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-09 13:48 [PATCH] drm/i915: Remove function details from device error messages Chris Wilson
2018-07-09 14:36 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-07-09 17:51 ` [PATCH] " Rodrigo Vivi
2018-07-09 20:14 ` Chris Wilson
2018-07-09 21:13 ` Rodrigo Vivi
2018-07-09 20:57 ` ✗ Fi.CI.IGT: failure for " 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.