From: "Alex Bennée" <alex.bennee@linaro.org> To: qemu-devel@nongnu.org Cc: "open list:PowerPC TCG CPUs" <qemu-ppc@nongnu.org>, 1836558@bugs.launchpad.net, "Alex Bennée" <alex.bennee@linaro.org>, "David Gibson" <david@gibson.dropbear.id.au> Subject: [Qemu-devel] [RFC PATCH for 4.1?] target/ppc: move opcode decode tables to PowerPCCPU Date: Tue, 16 Jul 2019 13:13:52 +0100 [thread overview] Message-ID: <20190716121352.302-1-alex.bennee@linaro.org> (raw) The opcode decode tables aren't really part of the CPUPPCState but an internal implementation detail for the translator. This can cause problems with memcpy in cpu_copy as any table created during ppc_cpu_realize get written over causing a memory leak. To avoid this move the tables into PowerPCCPU which is better suited to hold internal implementation details. Attempts to fix: https://bugs.launchpad.net/qemu/+bug/1836558 Cc: 1836558@bugs.launchpad.net Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- target/ppc/cpu.h | 8 ++++---- target/ppc/translate.c | 3 ++- target/ppc/translate_init.inc.c | 16 +++++++--------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index c9beba2a5c0..10e34b69b75 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1104,10 +1104,6 @@ struct CPUPPCState { bool resume_as_sreset; #endif - /* Those resources are used only during code translation */ - /* opcode handlers */ - opc_handler_t *opcodes[PPC_CPU_OPCODES_LEN]; - /* Those resources are used only in QEMU core */ target_ulong hflags; /* hflags is a MSR & HFLAGS_MASK */ target_ulong hflags_nmsr; /* specific hflags, not coming from MSR */ @@ -1191,6 +1187,10 @@ struct PowerPCCPU { int32_t node_id; /* NUMA node this CPU belongs to */ PPCHash64Options *hash64_opts; + /* Those resources are used only during code translation */ + /* opcode handlers */ + opc_handler_t *opcodes[PPC_CPU_OPCODES_LEN]; + /* Fields related to migration compatibility hacks */ bool pre_2_8_migration; target_ulong mig_msr_mask; diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 4a5de280365..c0faab8a824 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -7857,6 +7857,7 @@ static bool ppc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs, static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) { DisasContext *ctx = container_of(dcbase, DisasContext, base); + PowerPCCPU *cpu = POWERPC_CPU(cs); CPUPPCState *env = cs->env_ptr; opc_handler_t **table, *handler; @@ -7874,7 +7875,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) opc3(ctx->opcode), opc4(ctx->opcode), ctx->le_mode ? "little" : "big"); ctx->base.pc_next += 4; - table = env->opcodes; + table = cpu->opcodes; handler = table[opc1(ctx->opcode)]; if (is_indirect_opcode(handler)) { table = ind_table(handler); diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c index 86fc8f2e316..9cd2033bb92 100644 --- a/target/ppc/translate_init.inc.c +++ b/target/ppc/translate_init.inc.c @@ -9440,14 +9440,13 @@ static void fix_opcode_tables(opc_handler_t **ppc_opcodes) static void create_ppc_opcodes(PowerPCCPU *cpu, Error **errp) { PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); - CPUPPCState *env = &cpu->env; opcode_t *opc; - fill_new_table(env->opcodes, PPC_CPU_OPCODES_LEN); + fill_new_table(cpu->opcodes, PPC_CPU_OPCODES_LEN); for (opc = opcodes; opc < &opcodes[ARRAY_SIZE(opcodes)]; opc++) { if (((opc->handler.type & pcc->insns_flags) != 0) || ((opc->handler.type2 & pcc->insns_flags2) != 0)) { - if (register_insn(env->opcodes, opc) < 0) { + if (register_insn(cpu->opcodes, opc) < 0) { error_setg(errp, "ERROR initializing PowerPC instruction " "0x%02x 0x%02x 0x%02x", opc->opc1, opc->opc2, opc->opc3); @@ -9455,7 +9454,7 @@ static void create_ppc_opcodes(PowerPCCPU *cpu, Error **errp) } } } - fix_opcode_tables(env->opcodes); + fix_opcode_tables(cpu->opcodes); fflush(stdout); fflush(stderr); } @@ -10023,7 +10022,6 @@ static void ppc_cpu_unrealize(DeviceState *dev, Error **errp) { PowerPCCPU *cpu = POWERPC_CPU(dev); PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); - CPUPPCState *env = &cpu->env; Error *local_err = NULL; opc_handler_t **table, **table_2; int i, j, k; @@ -10035,11 +10033,11 @@ static void ppc_cpu_unrealize(DeviceState *dev, Error **errp) } for (i = 0; i < PPC_CPU_OPCODES_LEN; i++) { - if (env->opcodes[i] == &invalid_handler) { + if (cpu->opcodes[i] == &invalid_handler) { continue; } - if (is_indirect_opcode(env->opcodes[i])) { - table = ind_table(env->opcodes[i]); + if (is_indirect_opcode(cpu->opcodes[i])) { + table = ind_table(cpu->opcodes[i]); for (j = 0; j < PPC_CPU_INDIRECT_OPCODES_LEN; j++) { if (table[j] == &invalid_handler) { continue; @@ -10057,7 +10055,7 @@ static void ppc_cpu_unrealize(DeviceState *dev, Error **errp) ~PPC_INDIRECT)); } } - g_free((opc_handler_t *)((uintptr_t)env->opcodes[i] & + g_free((opc_handler_t *)((uintptr_t)cpu->opcodes[i] & ~PPC_INDIRECT)); } } -- 2.20.1
WARNING: multiple messages have this Message-ID (diff)
From: "Alex Bennée" <alex.bennee@linaro.org> To: qemu-devel@nongnu.org Subject: [Qemu-devel] [Bug 1836558] [RFC PATCH for 4.1?] target/ppc: move opcode decode tables to PowerPCCPU Date: Tue, 16 Jul 2019 12:13:52 -0000 [thread overview] Message-ID: <20190716121352.302-1-alex.bennee@linaro.org> (raw) Message-ID: <20190716121352.dul_hLPx1d-GWqa0ETWfiEThnaKAOYJceeV_thq0zpw@z> (raw) In-Reply-To: 156318593102.28533.3075291509963886255.malonedeb@chaenomeles.canonical.com The opcode decode tables aren't really part of the CPUPPCState but an internal implementation detail for the translator. This can cause problems with memcpy in cpu_copy as any table created during ppc_cpu_realize get written over causing a memory leak. To avoid this move the tables into PowerPCCPU which is better suited to hold internal implementation details. Attempts to fix: https://bugs.launchpad.net/qemu/+bug/1836558 Cc: 1836558@bugs.launchpad.net Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- target/ppc/cpu.h | 8 ++++---- target/ppc/translate.c | 3 ++- target/ppc/translate_init.inc.c | 16 +++++++--------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index c9beba2a5c0..10e34b69b75 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1104,10 +1104,6 @@ struct CPUPPCState { bool resume_as_sreset; #endif - /* Those resources are used only during code translation */ - /* opcode handlers */ - opc_handler_t *opcodes[PPC_CPU_OPCODES_LEN]; - /* Those resources are used only in QEMU core */ target_ulong hflags; /* hflags is a MSR & HFLAGS_MASK */ target_ulong hflags_nmsr; /* specific hflags, not coming from MSR */ @@ -1191,6 +1187,10 @@ struct PowerPCCPU { int32_t node_id; /* NUMA node this CPU belongs to */ PPCHash64Options *hash64_opts; + /* Those resources are used only during code translation */ + /* opcode handlers */ + opc_handler_t *opcodes[PPC_CPU_OPCODES_LEN]; + /* Fields related to migration compatibility hacks */ bool pre_2_8_migration; target_ulong mig_msr_mask; diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 4a5de280365..c0faab8a824 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -7857,6 +7857,7 @@ static bool ppc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs, static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) { DisasContext *ctx = container_of(dcbase, DisasContext, base); + PowerPCCPU *cpu = POWERPC_CPU(cs); CPUPPCState *env = cs->env_ptr; opc_handler_t **table, *handler; @@ -7874,7 +7875,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) opc3(ctx->opcode), opc4(ctx->opcode), ctx->le_mode ? "little" : "big"); ctx->base.pc_next += 4; - table = env->opcodes; + table = cpu->opcodes; handler = table[opc1(ctx->opcode)]; if (is_indirect_opcode(handler)) { table = ind_table(handler); diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c index 86fc8f2e316..9cd2033bb92 100644 --- a/target/ppc/translate_init.inc.c +++ b/target/ppc/translate_init.inc.c @@ -9440,14 +9440,13 @@ static void fix_opcode_tables(opc_handler_t **ppc_opcodes) static void create_ppc_opcodes(PowerPCCPU *cpu, Error **errp) { PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); - CPUPPCState *env = &cpu->env; opcode_t *opc; - fill_new_table(env->opcodes, PPC_CPU_OPCODES_LEN); + fill_new_table(cpu->opcodes, PPC_CPU_OPCODES_LEN); for (opc = opcodes; opc < &opcodes[ARRAY_SIZE(opcodes)]; opc++) { if (((opc->handler.type & pcc->insns_flags) != 0) || ((opc->handler.type2 & pcc->insns_flags2) != 0)) { - if (register_insn(env->opcodes, opc) < 0) { + if (register_insn(cpu->opcodes, opc) < 0) { error_setg(errp, "ERROR initializing PowerPC instruction " "0x%02x 0x%02x 0x%02x", opc->opc1, opc->opc2, opc->opc3); @@ -9455,7 +9454,7 @@ static void create_ppc_opcodes(PowerPCCPU *cpu, Error **errp) } } } - fix_opcode_tables(env->opcodes); + fix_opcode_tables(cpu->opcodes); fflush(stdout); fflush(stderr); } @@ -10023,7 +10022,6 @@ static void ppc_cpu_unrealize(DeviceState *dev, Error **errp) { PowerPCCPU *cpu = POWERPC_CPU(dev); PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); - CPUPPCState *env = &cpu->env; Error *local_err = NULL; opc_handler_t **table, **table_2; int i, j, k; @@ -10035,11 +10033,11 @@ static void ppc_cpu_unrealize(DeviceState *dev, Error **errp) } for (i = 0; i < PPC_CPU_OPCODES_LEN; i++) { - if (env->opcodes[i] == &invalid_handler) { + if (cpu->opcodes[i] == &invalid_handler) { continue; } - if (is_indirect_opcode(env->opcodes[i])) { - table = ind_table(env->opcodes[i]); + if (is_indirect_opcode(cpu->opcodes[i])) { + table = ind_table(cpu->opcodes[i]); for (j = 0; j < PPC_CPU_INDIRECT_OPCODES_LEN; j++) { if (table[j] == &invalid_handler) { continue; @@ -10057,7 +10055,7 @@ static void ppc_cpu_unrealize(DeviceState *dev, Error **errp) ~PPC_INDIRECT)); } } - g_free((opc_handler_t *)((uintptr_t)env->opcodes[i] & + g_free((opc_handler_t *)((uintptr_t)cpu->opcodes[i] & ~PPC_INDIRECT)); } } -- 2.20.1 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1836558 Title: Qemu-ppc Memory leak creating threads Status in QEMU: Confirmed Bug description: When creating c++ threads (with c++ std::thread), the resulting binary has memory leaks when running with qemu-ppc. Eg the following c++ program, when compiled with gcc, consumes more and more memory while running at qemu-ppc. (does not have memory leaks when compiling for Intel, when running same binary on real powerpc CPU hardware also no memory leaks). (Note I used function getCurrentRSS to show available memory, see https://stackoverflow.com/questions/669438/how-to-get-memory-usage-at- runtime-using-c; calls commented out here) Compiler: powerpc-linux-gnu-g++ (Debian 8.3.0-2) 8.3.0 (but same problem with older g++ compilers even 4.9) Os: Debian 10.0 ( Buster) (but same problem seen on Debian 9/stetch) qemu: qemu-ppc version 3.1.50 --- #include <iostream> #include <thread> #include <chrono> using namespace std::chrono_literals; // Create/run and join a 100 threads. void Fun100() { // auto b4 = getCurrentRSS(); // std::cout << getCurrentRSS() << std::endl; for(int n = 0; n < 100; n++) { std::thread t([] { std::this_thread::sleep_for( 10ms ); }); // std::cout << n << ' ' << getCurrentRSS() << std::endl; t.join(); } std::this_thread::sleep_for( 500ms ); // to give OS some time to wipe memory... // auto after = getCurrentRSS(); std::cout << b4 << ' ' << after << std::endl; } int main(int, char **) { Fun100(); Fun100(); // memory used keeps increasing } To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1836558/+subscriptions
next prev reply other threads:[~2019-07-16 12:14 UTC|newest] Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-15 10:18 [Qemu-devel] [Bug 1836558] [NEW] Qemu-ppc Memory leak creating threads Daan Scherft 2019-07-15 13:34 ` [Qemu-devel] [Bug 1836558] " Daan Scherft 2019-07-15 14:15 ` Alex Bennée 2019-07-15 15:10 ` Daan Scherft 2019-07-15 15:52 ` Alex Bennée 2019-07-15 16:21 ` Alex Bennée 2019-07-15 16:50 ` Alex Bennée 2019-07-16 12:13 ` Alex Bennée [this message] 2019-07-16 12:13 ` [Qemu-devel] [Bug 1836558] [RFC PATCH for 4.1?] target/ppc: move opcode decode tables to PowerPCCPU Alex Bennée 2019-07-16 14:50 ` [Qemu-devel] " Richard Henderson 2019-07-17 1:33 ` David Gibson 2019-07-17 9:41 ` Alex Bennée 2019-07-17 9:41 ` [Qemu-devel] [Bug 1836558] " Alex Bennée 2019-07-17 12:13 ` [Qemu-devel] " no-reply 2019-07-16 14:01 ` [Qemu-devel] [RFC PATCH for 4.1] linux-user: unparent CPU object before unref Alex Bennée 2019-07-16 14:01 ` [Qemu-devel] [Bug 1836558] " Alex Bennée 2019-07-16 14:43 ` [Qemu-devel] " Peter Maydell 2019-07-16 14:43 ` [Qemu-devel] [Bug 1836558] " Peter Maydell 2019-07-16 15:02 ` [Qemu-devel] " Alex Bennée 2019-07-16 15:02 ` [Qemu-devel] [Bug 1836558] " Alex Bennée 2019-07-16 15:17 ` [Qemu-devel] " Philippe Mathieu-Daudé 2020-01-09 13:22 ` [Bug 1836558] Re: Qemu-ppc Memory leak creating threads Thomas Huth 2020-03-10 8:48 ` Laurent Vivier
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190716121352.302-1-alex.bennee@linaro.org \ --to=alex.bennee@linaro.org \ --cc=1836558@bugs.launchpad.net \ --cc=david@gibson.dropbear.id.au \ --cc=qemu-devel@nongnu.org \ --cc=qemu-ppc@nongnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).