| Message ID | 20231009125137.1329146-5-george.dunlap@cloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xenalyze: Miscellaneous fixes | expand |
On Mon, Oct 09, 2023 at 01:51:36PM +0100, George Dunlap wrote: > EIP lists are generalized across several use cases. For many of them, > it make sense to have a cycle per sample; but not really for interrupt > EIP lists. For this reason, it normally just passes 0 as for the tsc > value, which will in turn down at the bottom of update_cycles(), > update only the summary.event_count, but nothing else. > > The dump_eip() function attempted to handle this by calling the generic > cycle print handler if the summary contained *any* cycles, and by collecting > and printing its own stats, based solely on counts, if not. > > Unfortunately, it used the wrong element for this: it collected the > total from samples.count rather samples.event_count; in the case that > there are no cycles, this will always be zero. It then divided by > this zero value. This results in output that looked like this: > > ``` > ffff89d29656 : 0 -nan% > ffff89d298b6 : 0 -nan% > ffff89d298c0 : 0 -nan% > ``` > > It's better than nothing, but a lot less informative than one would > like. > > Use event_count rather than count for collecting the total, and the > reporting when there are no cycles in the summary information. This results > in output that looks like this: > > ``` > ffff89d29656 : 2 1.21% > ffff89d298b6 : 1 0.61% > ffff89d298c0 : 1 0.61% > ``` > > Signed-off-by: George Dunlap <george.dunlap@cloud.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> Thanks,
diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c index 2faf66500d..4b6db59d87 100644 --- a/tools/xentrace/xenalyze.c +++ b/tools/xentrace/xenalyze.c @@ -2866,7 +2866,7 @@ void dump_eip(struct eip_list_struct *head) { for(p=head; p; p=p->next) { - total += p->summary.count; + total += p->summary.event_count; N++; } @@ -2901,8 +2901,8 @@ void dump_eip(struct eip_list_struct *head) { p->eip, find_symbol(p->eip)); printf(" %7d %5.2lf%%\n", - p->summary.count, - ((double)p->summary.count*100)/total); + p->summary.event_count, + ((double)p->summary.event_count*100)/total); }
EIP lists are generalized across several use cases. For many of them, it make sense to have a cycle per sample; but not really for interrupt EIP lists. For this reason, it normally just passes 0 as for the tsc value, which will in turn down at the bottom of update_cycles(), update only the summary.event_count, but nothing else. The dump_eip() function attempted to handle this by calling the generic cycle print handler if the summary contained *any* cycles, and by collecting and printing its own stats, based solely on counts, if not. Unfortunately, it used the wrong element for this: it collected the total from samples.count rather samples.event_count; in the case that there are no cycles, this will always be zero. It then divided by this zero value. This results in output that looked like this: ``` ffff89d29656 : 0 -nan% ffff89d298b6 : 0 -nan% ffff89d298c0 : 0 -nan% ``` It's better than nothing, but a lot less informative than one would like. Use event_count rather than count for collecting the total, and the reporting when there are no cycles in the summary information. This results in output that looks like this: ``` ffff89d29656 : 2 1.21% ffff89d298b6 : 1 0.61% ffff89d298c0 : 1 0.61% ``` Signed-off-by: George Dunlap <george.dunlap@cloud.com> --- Rationale: This is a bug fix. It's a minor one, but it's also in a non-critical part of the code. CC: Anthony Perard <anthony.perard@cloud.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Xenia Ragiodakou <xenia.ragiadakou@amd.com> --- tools/xentrace/xenalyze.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)