It adds a new structure which is linked with each TBs and stores its statistics. We collect the execution count of the TBs and store in this new structure. The information stored in this new struct is then used to support a new command line -d hot_tbs:N which dumps information of the N most hot TBs. Different from v1 now the execution count is being updated directly from the TBStatistics so we do not need to copy the data when flushing. [PATCH v2 1/4] add and link a statistic struct to TBs [PATCH v2 2/4] Adding an optional tb execution counter. [PATCH v2 3/4] Introduce dump of hot TBs [PATCH v2 4/4] adding -d hot_tbs:limit command line option
Patchew URL: https://patchew.org/QEMU/20190624055442.2973-1-vandersonmr2@gmail.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH v2 0/4] dumping hot TBs
Type: series
Message-id: 20190624055442.2973-1-vandersonmr2@gmail.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
From https://github.com/patchew-project/qemu
* [new tag] patchew/20190624055442.2973-1-vandersonmr2@gmail.com -> patchew/20190624055442.2973-1-vandersonmr2@gmail.com
Switched to a new branch 'test'
231c2807dd adding -d hot_tbs:limit command line option
d059715ded Introduce dump of hot TBs
25fe42cfad Adding an optional tb execution counter.
9d3e9b25e0 add and link a statistic struct to TBs
=== OUTPUT BEGIN ===
1/4 Checking commit 9d3e9b25e036 (add and link a statistic struct to TBs)
ERROR: trailing whitespace
#23: FILE: accel/tcg/translate-all.c:1121:
+static gint statistics_cmp(gconstpointer p1, gconstpointer p2) $
ERROR: trailing whitespace
#28: FILE: accel/tcg/translate-all.c:1126:
+ return (a->pc == b->pc && $
ERROR: code indent should never use tabs
#29: FILE: accel/tcg/translate-all.c:1127:
+^I^I a->cs_base == b->cs_base &&$
ERROR: trailing whitespace
#30: FILE: accel/tcg/translate-all.c:1128:
+^I^I a->flags == b->flags && $
ERROR: code indent should never use tabs
#30: FILE: accel/tcg/translate-all.c:1128:
+^I^I a->flags == b->flags && $
ERROR: code indent should never use tabs
#31: FILE: accel/tcg/translate-all.c:1129:
+^I a->page_addr[0] == b->page_addr[0] &&$
ERROR: trailing whitespace
#32: FILE: accel/tcg/translate-all.c:1130:
+ ^I a->page_addr[1] == b->page_addr[1]) ? 0 : 1; $
ERROR: code indent should never use tabs
#32: FILE: accel/tcg/translate-all.c:1130:
+ ^I a->page_addr[1] == b->page_addr[1]) ? 0 : 1; $
ERROR: open brace '{' following function declarations go on the next line
#42: FILE: accel/tcg/translate-all.c:1601:
+static void tb_insert_statistics_structure(TranslationBlock *tb) {
ERROR: line over 90 characters
#50: FILE: accel/tcg/translate-all.c:1609:
+ GList *lookup_result = g_list_find_custom(tb_ctx.tb_statistics, new_stats, statistics_cmp);
ERROR: code indent should never use tabs
#50: FILE: accel/tcg/translate-all.c:1609:
+^IGList *lookup_result = g_list_find_custom(tb_ctx.tb_statistics, new_stats, statistics_cmp);$
ERROR: code indent should never use tabs
#52: FILE: accel/tcg/translate-all.c:1611:
+^Iif (lookup_result) {$
WARNING: line over 80 characters
#53: FILE: accel/tcg/translate-all.c:1612:
+ /* If there is already a TBStatistic for this TB from a previous flush
ERROR: code indent should never use tabs
#53: FILE: accel/tcg/translate-all.c:1612:
+^I^I/* If there is already a TBStatistic for this TB from a previous flush$
WARNING: Block comments use a leading /* on a separate line
#53: FILE: accel/tcg/translate-all.c:1612:
+ /* If there is already a TBStatistic for this TB from a previous flush
ERROR: code indent should never use tabs
#54: FILE: accel/tcg/translate-all.c:1613:
+^I^I* then just make the new TB point to the older TBStatistic$
WARNING: Block comments should align the * on each line
#54: FILE: accel/tcg/translate-all.c:1613:
+ /* If there is already a TBStatistic for this TB from a previous flush
+ * then just make the new TB point to the older TBStatistic
ERROR: code indent should never use tabs
#55: FILE: accel/tcg/translate-all.c:1614:
+^I^I*/$
ERROR: code indent should never use tabs
#56: FILE: accel/tcg/translate-all.c:1615:
+^I^Ifree(new_stats);$
ERROR: code indent should never use tabs
#57: FILE: accel/tcg/translate-all.c:1616:
+ ^Itb->tb_stats = lookup_result->data;$
ERROR: code indent should never use tabs
#58: FILE: accel/tcg/translate-all.c:1617:
+^I} else {$
WARNING: line over 80 characters
#59: FILE: accel/tcg/translate-all.c:1618:
+ /* If not, then points to the new tb_statistics and add it to the hash */
ERROR: code indent should never use tabs
#59: FILE: accel/tcg/translate-all.c:1618:
+^I^I/* If not, then points to the new tb_statistics and add it to the hash */$
ERROR: code indent should never use tabs
#60: FILE: accel/tcg/translate-all.c:1619:
+^I^Itb->tb_stats = new_stats;$
ERROR: code indent should never use tabs
#61: FILE: accel/tcg/translate-all.c:1620:
+ ^Itb_ctx.tb_statistics = g_list_prepend(tb_ctx.tb_statistics, new_stats);$
ERROR: code indent should never use tabs
#62: FILE: accel/tcg/translate-all.c:1621:
+^I}$
ERROR: code indent should never use tabs
#73: FILE: accel/tcg/translate-all.c:1675:
+ ^I/* create and link to its TB a structure to store statistics */$
ERROR: code indent should never use tabs
#74: FILE: accel/tcg/translate-all.c:1676:
+ ^Itb_insert_statistics_structure(tb);$
ERROR: code indent should never use tabs
#75: FILE: accel/tcg/translate-all.c:1677:
+^I^I}$
ERROR: trailing whitespace
#88: FILE: include/exec/exec-all.h:327:
+typedef struct TBStatistics TBStatistics; $
ERROR: line over 90 characters
#88: FILE: include/exec/exec-all.h:327:
+typedef struct TBStatistics TBStatistics;
ERROR: trailing whitespace
#90: FILE: include/exec/exec-all.h:329:
+/* $
WARNING: line over 80 characters
#91: FILE: include/exec/exec-all.h:330:
+ * This struct stores statistics such as execution count of the TranslationBlocks.
ERROR: trailing whitespace
#92: FILE: include/exec/exec-all.h:331:
+ * Each TB has its own TBStatistics. TBStatistics is suppose to live even after $
ERROR: trailing whitespace
#95: FILE: include/exec/exec-all.h:334:
+struct TBStatistics { $
ERROR: line over 90 characters
#95: FILE: include/exec/exec-all.h:334:
+struct TBStatistics {
ERROR: trailing whitespace
#96: FILE: include/exec/exec-all.h:335:
+ target_ulong pc; $
ERROR: line over 90 characters
#96: FILE: include/exec/exec-all.h:335:
+ target_ulong pc;
ERROR: trailing whitespace
#97: FILE: include/exec/exec-all.h:336:
+ target_ulong cs_base; $
ERROR: line over 90 characters
#97: FILE: include/exec/exec-all.h:336:
+ target_ulong cs_base;
ERROR: trailing whitespace
#98: FILE: include/exec/exec-all.h:337:
+ uint32_t flags; $
ERROR: line over 90 characters
#98: FILE: include/exec/exec-all.h:337:
+ uint32_t flags;
ERROR: trailing whitespace
#99: FILE: include/exec/exec-all.h:338:
+ tb_page_addr_t page_addr[2]; $
ERROR: line over 90 characters
#99: FILE: include/exec/exec-all.h:338:
+ tb_page_addr_t page_addr[2];
ERROR: trailing whitespace
#101: FILE: include/exec/exec-all.h:340:
+ // total number of times that the related TB have being executed $
ERROR: line over 90 characters
#101: FILE: include/exec/exec-all.h:340:
+ // total number of times that the related TB have being executed
ERROR: do not use C99 // comments
#101: FILE: include/exec/exec-all.h:340:
+ // total number of times that the related TB have being executed
ERROR: trailing whitespace
#102: FILE: include/exec/exec-all.h:341:
+ uint32_t exec_count; $
ERROR: line over 90 characters
#102: FILE: include/exec/exec-all.h:341:
+ uint32_t exec_count;
ERROR: trailing whitespace
#103: FILE: include/exec/exec-all.h:342:
+ uint32_t exec_count_overflows; $
ERROR: line over 90 characters
#103: FILE: include/exec/exec-all.h:342:
+ uint32_t exec_count_overflows;
ERROR: trailing whitespace
#104: FILE: include/exec/exec-all.h:343:
+}; $
ERROR: do not use C99 // comments
#114: FILE: include/exec/exec-all.h:425:
+ // Pointer to a struct where statistics from the TB is stored
total: 48 errors, 5 warnings, 105 lines checked
Patch 1/4 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/4 Checking commit 25fe42cfad08 (Adding an optional tb execution counter.)
ERROR: "(foo*)" should be "(foo *)"
#25: FILE: accel/tcg/tcg-runtime.c:173:
+ TranslationBlock *tb = (TranslationBlock*) ptr;
WARNING: line over 80 characters
#26: FILE: accel/tcg/tcg-runtime.c:174:
+ // if overflows, then reset the execution counter and increment the overflow counter
ERROR: do not use C99 // comments
#26: FILE: accel/tcg/tcg-runtime.c:174:
+ // if overflows, then reset the execution counter and increment the overflow counter
WARNING: line over 80 characters
#27: FILE: accel/tcg/tcg-runtime.c:175:
+ if (atomic_cmpxchg(&tb->tb_stats->exec_count, 0xFFFFFFFF, 0) == 0xFFFFFFFF) {
total: 2 errors, 2 warnings, 43 lines checked
Patch 2/4 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/4 Checking commit d059715deda0 (Introduce dump of hot TBs)
ERROR: line over 90 characters
#30: FILE: accel/tcg/translate-all.c:1252:
+ qemu_log("Execution Count: \t%lu\n\n", (uint64_t) (tbs->exec_count + tbs->exec_count_overflows*0xFFFFFFFF));
ERROR: spaces required around that '*' (ctx:VxV)
#30: FILE: accel/tcg/translate-all.c:1252:
+ qemu_log("Execution Count: \t%lu\n\n", (uint64_t) (tbs->exec_count + tbs->exec_count_overflows*0xFFFFFFFF));
^
ERROR: line over 90 characters
#33: FILE: accel/tcg/translate-all.c:1255:
+ TranslationBlock *tb = tb_gen_code(current_cpu, tbs->pc, tbs->cs_base, tbs->flags, cflags);
ERROR: trailing whitespace
#49: FILE: accel/tcg/translate-all.c:1300:
+static gint inverse_sort_tbs(gconstpointer p1, gconstpointer p2) $
ERROR: line over 90 characters
#53: FILE: accel/tcg/translate-all.c:1304:
+ uint64_t p1_count = (uint64_t) (tbs1->exec_count + tbs1->exec_count_overflows*0xFFFFFFFF);
ERROR: spaces required around that '*' (ctx:VxV)
#53: FILE: accel/tcg/translate-all.c:1304:
+ uint64_t p1_count = (uint64_t) (tbs1->exec_count + tbs1->exec_count_overflows*0xFFFFFFFF);
^
ERROR: line over 90 characters
#54: FILE: accel/tcg/translate-all.c:1305:
+ uint64_t p2_count = (uint64_t) (tbs2->exec_count + tbs2->exec_count_overflows*0xFFFFFFFF);
ERROR: spaces required around that '*' (ctx:VxV)
#54: FILE: accel/tcg/translate-all.c:1305:
+ uint64_t p2_count = (uint64_t) (tbs2->exec_count + tbs2->exec_count_overflows*0xFFFFFFFF);
^
ERROR: code indent should never use tabs
#66: FILE: accel/tcg/translate-all.c:1317:
+^I tb_dump_statistics((TBStatistics *) i->data);$
total: 9 errors, 0 warnings, 63 lines checked
Patch 3/4 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/4 Checking commit 231c2807dd78 (adding -d hot_tbs:limit command line option)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190624055442.2973-1-vandersonmr2@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
© 2016 - 2024 Red Hat, Inc.