This is the first series of patches related to the TCGCodeQuality GSoC project 
More at https://wiki.qemu.org/Features/TCGCodeQuality

It adds an option to instrument TBs and collects their execution frequency.
The execution frequency is then store/accumulated in an auxiliary structure
every time a tb_flush or a read happens. 

[Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter.
[Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events.
[Qemu-Devel][PATCH 3/3] Adding command line option to linux-user.

Patchew URL: https://patchew.org/QEMU/20190614135332.12777-1-vandersonmr2@gmail.com/



Hi,

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

Subject: [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency
Type: series
Message-id: 20190614135332.12777-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 ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/155912548463.2019004.3515830305299809902.stgit@bahia.lan -> patchew/155912548463.2019004.3515830305299809902.stgit@bahia.lan
 - [tag update]      patchew/156051774276.244890.8660277280145466396.stgit@bahia.lan -> patchew/156051774276.244890.8660277280145466396.stgit@bahia.lan
Switched to a new branch 'test'
6512387 Adding command line option to linux-user.
af52efd Saving counters between tb_flush events.
906c8ef Adding an optional tb execution counter.

=== OUTPUT BEGIN ===
1/3 Checking commit 906c8ef92aac (Adding an optional tb execution counter.)
ERROR: "(foo*)" should be "(foo *)"
#24: FILE: accel/tcg/tcg-runtime.c:173:
+    TranslationBlock* tb = (TranslationBlock*) ptr;

ERROR: do not initialise globals to 0 or NULL
#83: FILE: linux-user/main.c:61:
+bool enable_freq_count = false;

ERROR: do not initialise globals to 0 or NULL
#95: FILE: vl.c:193:
+bool enable_freq_count = false;

total: 3 errors, 0 warnings, 56 lines checked

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

2/3 Checking commit af52efd7842d (Saving counters between tb_flush events.)
ERROR: "foo* bar" should be "foo *bar"
#22: FILE: accel/tcg/translate-all.c:1121:
+static bool statistics_cmp(const void* ap, const void *bp) {

ERROR: open brace '{' following function declarations go on the next line
#22: FILE: accel/tcg/translate-all.c:1121:
+static bool statistics_cmp(const void* ap, const void *bp) {

ERROR: line over 90 characters
#35: FILE: accel/tcg/translate-all.c:1146:
+    qht_init(&tb_ctx.tb_statistics, statistics_cmp, CODE_GEN_HTABLE_SIZE, QHT_MODE_AUTO_RESIZE);

ERROR: "(foo*)" should be "(foo *)"
#70: FILE: accel/tcg/translate-all.c:1265:
+    uint64_t exec_freq = tb_get_and_reset_exec_freq((TranslationBlock*) p);

ERROR: "(foo*)" should be "(foo *)"
#190: FILE: include/qom/cpu.h:477:
+uint64_t tb_get_and_reset_exec_freq(struct TranslationBlock*);

total: 5 errors, 0 warnings, 146 lines checked

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

3/3 Checking commit 651238793473 (Adding command line option to linux-user.)
ERROR: externs should be avoided in .c files
#22: FILE: linux-user/exit.c:29:
+extern bool enable_freq_count;

total: 1 errors, 0 warnings, 32 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190614135332.12777-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

vandersonmr <vandersonmr2@gmail.com> writes:

> This is the first series of patches related to the TCGCodeQuality GSoC project
> More at https://wiki.qemu.org/Features/TCGCodeQuality
>
> It adds an option to instrument TBs and collects their execution frequency.
> The execution frequency is then store/accumulated in an auxiliary structure
> every time a tb_flush or a read happens.
>
> [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter.
> [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events.
> [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user.

Did you explicitly set the patch prefix to qemu-devel? You don't need
to, they are added by the mailing list software.

--
Alex Bennée

vandersonmr <vandersonmr2@gmail.com> writes:

> This is the first series of patches related to the TCGCodeQuality GSoC project
> More at https://wiki.qemu.org/Features/TCGCodeQuality
>
> It adds an option to instrument TBs and collects their execution frequency.
> The execution frequency is then store/accumulated in an auxiliary structure
> every time a tb_flush or a read happens.
>
> [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter.
> [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events.
> [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user.

One more thing:

  https://app.shippable.com/github/stsquad/qemu/runs/866/summary/console

The use of:

  uint64_t exec_freq;

breaks 32 bit builds as we violate ATOMIC_REG_SIZE. Maybe we can get
away with uint32_t? I guess we need more of an idea of the range of
these counters are likely to hit (and maybe detect overflow in the
helper?).

--
Alex Bennée