Re: [PATCH v9 00/13] TCG code quality tracking and perf integration

[no-reply@patchew.org]

Patchew URL: https://patchew.org/QEMU/20191007152839.30804-1-alex.bennee@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH  v9 00/13] TCG code quality tracking and perf integration
Message-id: 20191007152839.30804-1-alex.bennee@linaro.org
Type: series

=== 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/20191007152839.30804-1-alex.bennee@linaro.org -> patchew/20191007152839.30804-1-alex.bennee@linaro.org
Switched to a new branch 'test'
2344fa6 configure: remove the final bits of --profiler support
f20376c tb-stats: adding TBStatistics info into perf dump
3ce3a87 accel/tcg: adding integration with linux perf
f430d85 tb-stats: dump hot TBs at the end of the execution
acc3b84 Adding info [tb-list|tb] commands to HMP (WIP)
cdf6f72 tb-stats: reset the tracked TBs on a tb_flush
9fe7b93 monitor: adding tb_stats hmp command
1358a45 debug: add -d tb_stats to control TBStatistics collection:
c1bf3a8 accel: adding TB_JIT_TIME and full replacing CONFIG_PROFILER
947c81f accel: replacing part of CONFIG_PROFILER with TBStats
b3bf3ff accel: collecting JIT statistics
c7359e9 accel: collecting TB execution count
ac71d5f accel/tcg: introduce TBStatistics structure

=== OUTPUT BEGIN ===
1/13 Checking commit ac71d5f77f09 (accel/tcg: introduce TBStatistics structure)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#37: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#231: FILE: include/exec/tb-context.h:26:
+/* Page tracking code uses ram addresses in system mode, and virtual

WARNING: Block comments use * on subsequent lines
#232: FILE: include/exec/tb-context.h:27:
+/* Page tracking code uses ram addresses in system mode, and virtual
+   addresses in userspace mode.  Define tb_page_addr_t to be an appropriate

WARNING: Block comments use a trailing */ on a separate line
#233: FILE: include/exec/tb-context.h:28:
+   type.  */

total: 0 errors, 4 warnings, 271 lines checked

Patch 1/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/13 Checking commit c7359e966d69 (accel: collecting TB execution count)
3/13 Checking commit b3bf3ff3a8e4 (accel: collecting JIT statistics)
4/13 Checking commit 947c81f1a9c9 (accel: replacing part of CONFIG_PROFILER with TBStats)
WARNING: line over 80 characters
#105: FILE: accel/tcg/tb-stats.c:85:
+            qemu_printf("JIT cycles          %" PRId64 " (%0.3f s at 2.4 GHz)\n",

WARNING: line over 80 characters
#126: FILE: accel/tcg/tb-stats.c:106:
+                    (double)s->la_time / (s->code_time ? s->code_time : 1) * 100.0);

WARNING: line over 80 characters
#130: FILE: accel/tcg/tb-stats.c:110:
+                    s->restore_count ? (double)s->restore_time / s->restore_count : 0);

total: 0 errors, 3 warnings, 345 lines checked

Patch 4/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/13 Checking commit c1bf3a89e351 (accel: adding TB_JIT_TIME and full replacing CONFIG_PROFILER)
WARNING: line over 80 characters
#112: FILE: accel/tcg/tb-stats.c:142:
+                (double)jpi->opt_time / (jpi->code_time ? jpi->code_time : 1) * 100.0);

WARNING: line over 80 characters
#114: FILE: accel/tcg/tb-stats.c:144:
+                (double)jpi->la_time / (jpi->code_time ? jpi->code_time : 1) * 100.0);

WARNING: line over 80 characters
#118: FILE: accel/tcg/tb-stats.c:148:
+                jpi->restore_count ? (double)jpi->restore_time / jpi->restore_count : 0);

WARNING: line over 80 characters
#149: FILE: accel/tcg/tb-stats.c:152:
+                s->cpu_exec_time, s->cpu_exec_time / (double) NANOSECONDS_PER_SECOND);

WARNING: line over 80 characters
#589: FILE: tcg/tcg.c:4133:
+        qemu_printf("%s %" PRId64 "\n", tcg_op_defs[i].name, prof.table_op_count[i]);

total: 0 errors, 5 warnings, 557 lines checked

Patch 5/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/13 Checking commit 1358a45e85db (debug: add -d tb_stats to control TBStatistics collection:)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#53: 
new file mode 100644

WARNING: line over 80 characters
#206: FILE: util/log.c:279:
+    { CPU_LOG_TB_STATS, "tb_stats[[,level=(+all+jit+exec+time)][,dump_limit=<number>]]",

WARNING: line over 80 characters
#218: FILE: util/log.c:305:
+            set_default_tbstats_flag(TB_JIT_STATS | TB_EXEC_STATS | TB_JIT_TIME);

WARNING: line over 80 characters
#228: FILE: util/log.c:315:
+            for (level_tmp = level_parts; level_tmp && *level_tmp; level_tmp++) {

total: 0 errors, 4 warnings, 176 lines checked

Patch 6/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/13 Checking commit 9fe7b93cf6d7 (monitor: adding tb_stats hmp command)
WARNING: line over 80 characters
#103: FILE: accel/tcg/tb-stats.c:201:
+        /* Continue to create TBStatistic structures but stop collecting statistics */

WARNING: line over 80 characters
#126: FILE: accel/tcg/tb-stats.c:224:
+            qemu_printf("no search on record! execute info tbs before filtering!");

WARNING: line over 80 characters
#132: FILE: accel/tcg/tb-stats.c:230:
+        /* Set all tbstats as paused, then return only the ones from last_search */

WARNING: Block comments use a leading /* on a separate line
#223: FILE: include/exec/tb-stats.h:35:
+enum SortBy { SORT_BY_HOTNESS, SORT_BY_HG /* Host/Guest */, SORT_BY_SPILLS };

total: 0 errors, 4 warnings, 259 lines checked

Patch 7/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/13 Checking commit cdf6f7219143 (tb-stats: reset the tracked TBs on a tb_flush)
9/13 Checking commit acc3b8494d38 (Adding info [tb-list|tb] commands to HMP (WIP))
WARNING: line over 80 characters
#144: FILE: accel/tcg/tb-stats.c:325:
+        qemu_log("\t| exec:%lu/%lu guest inst cov:%.2f%%\n", tbs->executions.normal,

WARNING: line over 80 characters
#188: FILE: accel/tcg/tb-stats.c:369:
+            (float) stat_per_translation(tbs1, code.out_len) / tbs1->code.num_guest_inst;

WARNING: line over 80 characters
#190: FILE: accel/tcg/tb-stats.c:371:
+            (float) stat_per_translation(tbs2, code.out_len) / tbs2->code.num_guest_inst;

WARNING: line over 80 characters
#223: FILE: accel/tcg/tb-stats.c:404:
+            (tbs->executions.atomic + tbs->executions.normal) * tbs->code.num_guest_inst;

WARNING: line over 80 characters
#229: FILE: accel/tcg/tb-stats.c:410:
+            (tbs->executions.atomic + tbs->executions.normal) * tbs->code.num_guest_inst;

WARNING: line over 80 characters
#230: FILE: accel/tcg/tb-stats.c:411:
+        tbs->executions.coverage = (10000 * tb_total_execs) / (total_exec_count + 1);

WARNING: line over 80 characters
#247: FILE: accel/tcg/tb-stats.c:428:
+    last_search = g_list_sort_with_data(last_search, inverse_sort_tbs, &sort_by);

total: 0 errors, 7 warnings, 632 lines checked

Patch 9/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/13 Checking commit f430d85d6d0a (tb-stats: dump hot TBs at the end of the execution)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#90: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#112: FILE: include/exec/tb-stats-dump.h:18:
+static inline void tb_stats_dump(void) { /* do nothing */ };

total: 0 errors, 2 warnings, 123 lines checked

Patch 10/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/13 Checking commit 3ce3a87a1f50 (accel/tcg: adding integration with linux perf)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#38: 
new file mode 100644

WARNING: line over 80 characters
#51: FILE: accel/tcg/perf/jitdump.c:2:
+ * This code implements an interface to create and fill jitdump files. These files

WARNING: line over 80 characters
#161: FILE: accel/tcg/perf/jitdump.c:112:
+    g_autofree gchar *dumpfile_name = g_strdup_printf("./jit-%d.dump", getpid());

WARNING: Block comments use a leading /* on a separate line
#164: FILE: accel/tcg/perf/jitdump.c:115:
+    /* 'Perf record' saves mmaped files during the execution of a program and

WARNING: line over 80 characters
#175: FILE: accel/tcg/perf/jitdump.c:126:
+        printf("Failed to create mmap marker file for perf %d\n", fileno(dumpfile));

WARNING: architecture specific defines should be avoided
#294: FILE: accel/tcg/translate-all.c:61:
+#ifdef __linux__

WARNING: architecture specific defines should be avoided
#305: FILE: accel/tcg/translate-all.c:1174:
+#ifdef __linux__

WARNING: architecture specific defines should be avoided
#317: FILE: accel/tcg/translate-all.c:1990:
+#ifdef __linux__

WARNING: line over 80 characters
#381: FILE: linux-user/main.c:458:
+     "",           "dump jitdump files to help linux perf JIT code visualization"},

ERROR: space prohibited between function name and open parenthesis '('
#393: FILE: os-posix.c:194:
+#if defined(CONFIG_TCG) && defined (CLOCK_MONOTONIC)

total: 1 errors, 9 warnings, 342 lines checked

Patch 11/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

12/13 Checking commit f20376c59d71 (tb-stats: adding TBStatistics info into perf dump)
13/13 Checking commit 2344fa699da4 (configure: remove the final bits of --profiler support)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20191007152839.30804-1-alex.bennee@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com