* [PATCH i-g-t] intel_l3_parity: More helpful output in case of errors
@ 2017-09-05 12:39 Petri Latvala
2017-09-05 12:56 ` ✗ Fi.CI.BAT: warning for " Patchwork
2017-09-05 16:16 ` [PATCH i-g-t] " Daniel Vetter
0 siblings, 2 replies; 6+ messages in thread
From: Petri Latvala @ 2017-09-05 12:39 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
When no action is specified on the command line, print the usage help
text and exit with failure instead of SIGABRT. Fix some typos on the
usage text.
Keep the abort() call in places where they can only be reached by
expanding the tool and forgetting to handle new parameters, with an
error message printed.
CC: Ben Widawsky <benjamin.widawsky@intel.com>
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
---
tools/intel_l3_parity.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
index eb00c50..1a4fae5 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -172,9 +172,9 @@ static void usage(const char *name)
" -l, --list List the current L3 logs\n"
" -a, --clear-all Clear all disabled rows\n"
" -e, --enable Enable row, bank, subbank (undo -d)\n"
- " -d, --disable=<row,bank,subbank> Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead\n"
- " -i, --inject [HSW only] Cause hardware to inject a row errors\n"
- " -u, --uninject [HSW only] Turn off hardware error injectection (undo -i)\n"
+ " -d, --disable=<row,bank,subbank> Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead)\n"
+ " -i, --inject [HSW only] Cause hardware to inject a row error\n"
+ " -u, --uninject [HSW only] Turn off hardware error injection (undo -i)\n"
" -L, --listen Listen for uevent errors\n",
name);
}
@@ -301,6 +301,7 @@ int main(int argc, char *argv[])
action = c;
break;
default:
+ fprintf(stderr, "Internal error: Unhandled flag %c\n", c);
abort();
}
}
@@ -374,7 +375,12 @@ int main(int argc, char *argv[])
break;
case 'L':
break;
+ case '0':
+ /* No action given */
+ usage(argv[0]);
+ exit(EXIT_FAILURE);
default:
+ fprintf(stderr, "Internal error: Unhandled action %d\n", action);
abort();
}
}
--
2.9.3
_______________________________________________
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: warning for intel_l3_parity: More helpful output in case of errors
2017-09-05 12:39 [PATCH i-g-t] intel_l3_parity: More helpful output in case of errors Petri Latvala
@ 2017-09-05 12:56 ` Patchwork
2017-09-05 16:16 ` [PATCH i-g-t] " Daniel Vetter
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-09-05 12:56 UTC (permalink / raw)
To: Petri Latvala; +Cc: intel-gfx
== Series Details ==
Series: intel_l3_parity: More helpful output in case of errors
URL : https://patchwork.freedesktop.org/series/29824/
State : warning
== Summary ==
IGT patchset tested on top of latest successful build
918863f8e3e8f49235fd2e4a36e11f386c06c11c intel_display_poller: Fix truncation of a test name.
with latest DRM-Tip kernel build CI_DRM_3039
229aae71d40f drm-tip: 2017y-09m-05d-11h-05m-56s UTC integration manifest
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
pass -> FAIL (fi-snb-2600) fdo#100215
Subgroup basic-flip-after-cursor-atomic:
pass -> DMESG-WARN (fi-glk-2a)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> SKIP (fi-skl-x1585l) fdo#101781
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> INCOMPLETE (fi-bsw-n3050) fdo#101707
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101707 https://bugs.freedesktop.org/show_bug.cgi?id=101707
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:464s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:439s
fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:363s
fi-bsw-n3050 total:225 pass:195 dwarn:0 dfail:0 fail:0 skip:29
fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:254s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:530s
fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:524s
fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:522s
fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:439s
fi-glk-2a total:289 pass:259 dwarn:1 dfail:0 fail:0 skip:29 time:622s
fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:450s
fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:428s
fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:430s
fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:515s
fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:477s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:501s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:599s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:603s
fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:533s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:475s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:536s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:524s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:452s
fi-skl-x1585l total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:500s
fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:565s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:1 skip:39 time:405s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_146/
_______________________________________________
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 i-g-t] intel_l3_parity: More helpful output in case of errors
2017-09-05 12:39 [PATCH i-g-t] intel_l3_parity: More helpful output in case of errors Petri Latvala
2017-09-05 12:56 ` ✗ Fi.CI.BAT: warning for " Patchwork
@ 2017-09-05 16:16 ` Daniel Vetter
2017-09-05 17:14 ` Ben Widawsky
2017-09-06 9:31 ` Petri Latvala
1 sibling, 2 replies; 6+ messages in thread
From: Daniel Vetter @ 2017-09-05 16:16 UTC (permalink / raw)
To: Petri Latvala; +Cc: intel-gfx, Ben Widawsky
On Tue, Sep 05, 2017 at 03:39:49PM +0300, Petri Latvala wrote:
> When no action is specified on the command line, print the usage help
> text and exit with failure instead of SIGABRT. Fix some typos on the
> usage text.
>
> Keep the abort() call in places where they can only be reached by
> expanding the tool and forgetting to handle new parameters, with an
> error message printed.
>
> CC: Ben Widawsky <benjamin.widawsky@intel.com>
> Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> ---
> tools/intel_l3_parity.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
> index eb00c50..1a4fae5 100644
> --- a/tools/intel_l3_parity.c
> +++ b/tools/intel_l3_parity.c
> @@ -172,9 +172,9 @@ static void usage(const char *name)
> " -l, --list List the current L3 logs\n"
> " -a, --clear-all Clear all disabled rows\n"
> " -e, --enable Enable row, bank, subbank (undo -d)\n"
> - " -d, --disable=<row,bank,subbank> Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead\n"
> - " -i, --inject [HSW only] Cause hardware to inject a row errors\n"
> - " -u, --uninject [HSW only] Turn off hardware error injectection (undo -i)\n"
> + " -d, --disable=<row,bank,subbank> Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead)\n"
> + " -i, --inject [HSW only] Cause hardware to inject a row error\n"
> + " -u, --uninject [HSW only] Turn off hardware error injection (undo -i)\n"
> " -L, --listen Listen for uevent errors\n",
> name);
> }
> @@ -301,6 +301,7 @@ int main(int argc, char *argv[])
> action = c;
> break;
> default:
> + fprintf(stderr, "Internal error: Unhandled flag %c\n", c);
> abort();
> }
> }
> @@ -374,7 +375,12 @@ int main(int argc, char *argv[])
> break;
> case 'L':
> break;
> + case '0':
> + /* No action given */
> + usage(argv[0]);
> + exit(EXIT_FAILURE);
Won't this print usage once per slice? Or am I misreading how the patch
applies ...
-Daniel
> default:
> + fprintf(stderr, "Internal error: Unhandled action %d\n", action);
> abort();
> }
> }
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
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 i-g-t] intel_l3_parity: More helpful output in case of errors
2017-09-05 16:16 ` [PATCH i-g-t] " Daniel Vetter
@ 2017-09-05 17:14 ` Ben Widawsky
2017-09-06 9:31 ` Petri Latvala
1 sibling, 0 replies; 6+ messages in thread
From: Ben Widawsky @ 2017-09-05 17:14 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On 17-09-05 18:16:23, Daniel Vetter wrote:
>On Tue, Sep 05, 2017 at 03:39:49PM +0300, Petri Latvala wrote:
>> When no action is specified on the command line, print the usage help
>> text and exit with failure instead of SIGABRT. Fix some typos on the
>> usage text.
>>
>> Keep the abort() call in places where they can only be reached by
>> expanding the tool and forgetting to handle new parameters, with an
>> error message printed.
>>
>> CC: Ben Widawsky <benjamin.widawsky@intel.com>
>> Signed-off-by: Petri Latvala <petri.latvala@intel.com>
>> ---
>> tools/intel_l3_parity.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
>> index eb00c50..1a4fae5 100644
>> --- a/tools/intel_l3_parity.c
>> +++ b/tools/intel_l3_parity.c
>> @@ -172,9 +172,9 @@ static void usage(const char *name)
>> " -l, --list List the current L3 logs\n"
>> " -a, --clear-all Clear all disabled rows\n"
>> " -e, --enable Enable row, bank, subbank (undo -d)\n"
>> - " -d, --disable=<row,bank,subbank> Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead\n"
>> - " -i, --inject [HSW only] Cause hardware to inject a row errors\n"
>> - " -u, --uninject [HSW only] Turn off hardware error injectection (undo -i)\n"
>> + " -d, --disable=<row,bank,subbank> Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead)\n"
>> + " -i, --inject [HSW only] Cause hardware to inject a row error\n"
>> + " -u, --uninject [HSW only] Turn off hardware error injection (undo -i)\n"
+1 for fixing the typo.
I think we should be careful about using "HSW only". In fact I'm in favor of
removing what was already there because I honestly don't know which platforms
support this feature.
>> " -L, --listen Listen for uevent errors\n",
>> name);
>> }
>> @@ -301,6 +301,7 @@ int main(int argc, char *argv[])
>> action = c;
>> break;
>> default:
>> + fprintf(stderr, "Internal error: Unhandled flag %c\n", c);
>> abort();
>> }
>> }
>> @@ -374,7 +375,12 @@ int main(int argc, char *argv[])
>> break;
>> case 'L':
>> break;
>> + case '0':
>> + /* No action given */
>> + usage(argv[0]);
>> + exit(EXIT_FAILURE);
>
>Won't this print usage once per slice? Or am I misreading how the patch
>applies ...
>-Daniel
>
Looks wrong to me as well.
>> default:
>> + fprintf(stderr, "Internal error: Unhandled action %d\n", action);
>> abort();
>> }
>> }
>> --
>> 2.9.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
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 i-g-t] intel_l3_parity: More helpful output in case of errors
2017-09-05 16:16 ` [PATCH i-g-t] " Daniel Vetter
2017-09-05 17:14 ` Ben Widawsky
@ 2017-09-06 9:31 ` Petri Latvala
2017-09-08 6:57 ` Daniel Vetter
1 sibling, 1 reply; 6+ messages in thread
From: Petri Latvala @ 2017-09-06 9:31 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Ben Widawsky
On 09/05/2017 07:16 PM, Daniel Vetter wrote:
> On Tue, Sep 05, 2017 at 03:39:49PM +0300, Petri Latvala wrote:
>> When no action is specified on the command line, print the usage help
>> text and exit with failure instead of SIGABRT. Fix some typos on the
>> usage text.
>>
>> Keep the abort() call in places where they can only be reached by
>> expanding the tool and forgetting to handle new parameters, with an
>> error message printed.
>>
>> CC: Ben Widawsky <benjamin.widawsky@intel.com>
>> Signed-off-by: Petri Latvala <petri.latvala@intel.com>
>> ---
>> tools/intel_l3_parity.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
>> index eb00c50..1a4fae5 100644
>> --- a/tools/intel_l3_parity.c
>> +++ b/tools/intel_l3_parity.c
>> @@ -172,9 +172,9 @@ static void usage(const char *name)
>> " -l, --list List the current L3 logs\n"
>> " -a, --clear-all Clear all disabled rows\n"
>> " -e, --enable Enable row, bank, subbank (undo -d)\n"
>> - " -d, --disable=<row,bank,subbank> Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead\n"
>> - " -i, --inject [HSW only] Cause hardware to inject a row errors\n"
>> - " -u, --uninject [HSW only] Turn off hardware error injectection (undo -i)\n"
>> + " -d, --disable=<row,bank,subbank> Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead)\n"
>> + " -i, --inject [HSW only] Cause hardware to inject a row error\n"
>> + " -u, --uninject [HSW only] Turn off hardware error injection (undo -i)\n"
>> " -L, --listen Listen for uevent errors\n",
>> name);
>> }
>> @@ -301,6 +301,7 @@ int main(int argc, char *argv[])
>> action = c;
>> break;
>> default:
>> + fprintf(stderr, "Internal error: Unhandled flag %c\n", c);
>> abort();
>> }
>> }
>> @@ -374,7 +375,12 @@ int main(int argc, char *argv[])
>> break;
>> case 'L':
>> break;
>> + case '0':
>> + /* No action given */
>> + usage(argv[0]);
>> + exit(EXIT_FAILURE);
> Won't this print usage once per slice? Or am I misreading how the patch
> applies ...
It prints the usage and calls exit(), what is the control flow that
leads to printing it multiple times?
--
Petri Latvala
_______________________________________________
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 i-g-t] intel_l3_parity: More helpful output in case of errors
2017-09-06 9:31 ` Petri Latvala
@ 2017-09-08 6:57 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2017-09-08 6:57 UTC (permalink / raw)
To: Petri Latvala; +Cc: intel-gfx, Ben Widawsky
On Wed, Sep 06, 2017 at 12:31:05PM +0300, Petri Latvala wrote:
>
>
> On 09/05/2017 07:16 PM, Daniel Vetter wrote:
> > On Tue, Sep 05, 2017 at 03:39:49PM +0300, Petri Latvala wrote:
> > > When no action is specified on the command line, print the usage help
> > > text and exit with failure instead of SIGABRT. Fix some typos on the
> > > usage text.
> > >
> > > Keep the abort() call in places where they can only be reached by
> > > expanding the tool and forgetting to handle new parameters, with an
> > > error message printed.
> > >
> > > CC: Ben Widawsky <benjamin.widawsky@intel.com>
> > > Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> > > ---
> > > tools/intel_l3_parity.c | 12 +++++++++---
> > > 1 file changed, 9 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
> > > index eb00c50..1a4fae5 100644
> > > --- a/tools/intel_l3_parity.c
> > > +++ b/tools/intel_l3_parity.c
> > > @@ -172,9 +172,9 @@ static void usage(const char *name)
> > > " -l, --list List the current L3 logs\n"
> > > " -a, --clear-all Clear all disabled rows\n"
> > > " -e, --enable Enable row, bank, subbank (undo -d)\n"
> > > - " -d, --disable=<row,bank,subbank> Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead\n"
> > > - " -i, --inject [HSW only] Cause hardware to inject a row errors\n"
> > > - " -u, --uninject [HSW only] Turn off hardware error injectection (undo -i)\n"
> > > + " -d, --disable=<row,bank,subbank> Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead)\n"
> > > + " -i, --inject [HSW only] Cause hardware to inject a row error\n"
> > > + " -u, --uninject [HSW only] Turn off hardware error injection (undo -i)\n"
> > > " -L, --listen Listen for uevent errors\n",
> > > name);
> > > }
> > > @@ -301,6 +301,7 @@ int main(int argc, char *argv[])
> > > action = c;
> > > break;
> > > default:
> > > + fprintf(stderr, "Internal error: Unhandled flag %c\n", c);
> > > abort();
> > > }
> > > }
> > > @@ -374,7 +375,12 @@ int main(int argc, char *argv[])
> > > break;
> > > case 'L':
> > > break;
> > > + case '0':
> > > + /* No action given */
> > > + usage(argv[0]);
> > > + exit(EXIT_FAILURE);
> > Won't this print usage once per slice? Or am I misreading how the patch
> > applies ...
>
>
> It prints the usage and calls exit(), what is the control flow that leads to
> printing it multiple times?
Ah, that's indeed a bit confusing control flow that usage() exists. Would
be cleaner if we don't hide the call somewhere in a loop, but bail out
more top-level.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
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:[~2017-09-08 6:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-05 12:39 [PATCH i-g-t] intel_l3_parity: More helpful output in case of errors Petri Latvala
2017-09-05 12:56 ` ✗ Fi.CI.BAT: warning for " Patchwork
2017-09-05 16:16 ` [PATCH i-g-t] " Daniel Vetter
2017-09-05 17:14 ` Ben Widawsky
2017-09-06 9:31 ` Petri Latvala
2017-09-08 6:57 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox