Thanks Ali. I’ll use Mutex as the suffix instead of Mtx. I’ll also test out some other solutions for the decoder issue. Thanks for pointing me to the 5th options as well. I will check it out.
From: Ali Saidi [mailto:[email protected]] On Behalf Of Ali Saidi Sent: Friday, August 01, 2014 3:47 PM To: Brown, Martin; Ali Saidi; Default Subject: Re: Review Request 2320: sim: stopgap for race-conditions when using multiple EventQueues This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2320/ Thanks. I think the non decoder stuff is fine to commit, but I'm not sure how we go about selecting a long term solution src/arch/x86/decoder.cc<http://reviews.gem5.org/r/2320/diff/1/?file=40450#file40450line463> (Diff revision 1) Decoder::doImmediateState() 463 std::mutex decodeMtx; You've used Mtx and Mutex as a suffix for a lock. I don't know that we have a particular coding style, but in the places we've used a mutex in the code so far we used mutex, so unless there are any strong objections I vote to stick with that src/arch/x86/decoder.cc<http://reviews.gem5.org/r/2320/diff/1/?file=40450#file40450line470> (Diff revision 1) Decoder::doImmediateState() 470 // concurrent readers on instMap. Alternative solutions include: There is a 5th option: A thread-local instCachMap which is probably superior to #1 assuming you have more that one simulated cpu per physical cpu. There is shared_lock in c++14 and the implementation is pretty simple based on mutex and condition variables: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2406.html#shared_mutex - Ali Saidi On August 1st, 2014, 7:04 p.m. UTC, Martin Brown wrote: Review request for Default. By Martin Brown. Updated Aug. 1, 2014, 7:04 p.m. Repository: gem5 Description Changeset 10264:c3977836244e --------------------------- sim: stopgap for race-conditions when using multiple EventQueues This patch fixes several race conditions that appear in multi- threaded mode. Currently the decode cache race condition is fixed only for x86, and in a temporary non-optimal fashion. We still need to decide on a more optimal solution for the decode cache and apply it to all the ISAs. Testing - Quick regression tests on x86, arm, alpha - Made sure that sparc, power, mips can be built with this patch - Tested using up to 28 EventQueues (28 threads) Diffs * src/arch/x86/decoder.cc (c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76) * src/base/refcnt.hh (c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76) * src/base/trace.cc (c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76) * src/sim/syscall_emul.cc (c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76) View Diff<http://reviews.gem5.org/r/2320/diff/> _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
