Re: [Qemu-devel] [PATCH v2 1/2] accel/tcg: adding integration with linux perf

[Alex Bennée]

Stefan Hajnoczi <address@hidden> writes:

> On Fri, Aug 30, 2019 at 09:19:02AM -0300, vandersonmr wrote:
>> This commit adds support to Linux Perf in order
>> to be able to analyze qemu jitted code and
>> also to able to see the TBs PC in it.
>>
>> When using "-perf" qemu creates a jitdump file in
>> the current working directory. The file format
>> specification can be found in:
>> https://github.com/torvalds/linux/blob/master/tools/perf/Documentation/jitdump-specification.tx
>
> Oops, the link is broken: .txt
>
>> +struct jr_code_close {
>> +    struct jr_prefix p;
>> +};
>
> Unused?
>
>> +
>> +struct jr_code_move {
>
> Unused?
>
>> +static uint32_t get_e_machine(void)
>> +{
>> +    uint32_t e_machine = EM_NONE;
>> +    Elf64_Ehdr elf_header;
>> +    FILE *exe = fopen("/proc/self/exe", "r");
>> +
>> +    if (exe == NULL) {
>> +        return e_machine;
>> +    }
>> +
>> +    if (fread(&elf_header, sizeof(Elf64_Ehdr), 1, exe) != 1) {
>
> What if this is a 32-bit binary because QEMU was built for a 32-bit
> host?
>
>> +        goto end;
>> +    }
>> +
>> +    e_machine = elf_header.e_machine;
>> +
>> +end:
>> +    fclose(exe);
>> +    return e_machine;
>> +}
>> +
>> +void start_jitdump_file(void)
>> +{
>> +    gchar *dumpfile_name = g_strdup_printf("./jit-%d.dump", getpid());
>
> You can now use g_autofree:
>
>   g_autofree gchar *dumpfile_name = g_strdup_printf(...);
>
> and then the explicit g_free() isn't necessary anymore (and the memory
> leak in the mmap error case is also solved).
>
>> +void append_load_in_jitdump_file(TranslationBlock *tb)
>> +{
>> +    gchar *func_name = g_strdup_printf("TB virt:0x"TARGET_FMT_lx, tb->pc);
>> +
>> +    struct jr_code_load load_event;
>> +    load_event.p.id = JIT_CODE_LOAD;
>> +    load_event.p.total_size =
>> +        sizeof(struct jr_code_load) + strlen(func_name) + 1 + tb->tc.size;
>> +    load_event.p.timestamp = get_timestamp();
>> +    load_event.pid = getpid();
>> +    load_event.tid = syscall(SYS_gettid);
>> +    load_event.vma = tb->pc;
>> +    load_event.code_addr = (uint64_t) tb->tc.ptr;
>> +    load_event.code_size = tb->tc.size;
>> +    load_event.code_index = tb->pc;
>> +
>> +    fwrite(&load_event, sizeof(struct jr_code_load), 1, dumpfile);
>> +    fwrite(func_name, strlen(func_name) + 1, 1, dumpfile);
>> +    fwrite(tb->tc.ptr, tb->tc.size, 1, dumpfile);
>> +
>> +    g_free(func_name);
>> +    fflush(dumpfile);
>> +}
>
> I didn't see a reply to my question on the previous patch series:
>
>   "append_load_in_jitdump_file() calls fwrite() multiple times.  What
>   guarantees they will not be interleaved when multiple threads call
>   this function?"

Ahh that was exactly the problem I was debugging. Fixed with a lock now.

> Does TCG ever throw away TBs and is it necessary to record this in the
> file so perf knows about it?

Yes - I suspect that's what the currently unused headers are for.

--
Alex Bennée