Emilio G. Cota <[email protected]> writes: > On Fri, Jul 01, 2016 at 17:16:10 +0100, Alex Bennée wrote: >> Lock contention in the hot path of moving between existing patched >> TranslationBlocks is the main drag in multithreaded performance. This >> patch pushes the tb_lock() usage down to the two places that really need >> it: >> >> - code generation (tb_gen_code) >> - jump patching (tb_add_jump) >> >> The rest of the code doesn't really need to hold a lock as it is either >> using per-CPU structures, atomically updated or designed to be used in >> concurrent read situations (qht_lookup). >> >> To keep things simple I removed the #ifdef CONFIG_USER_ONLY stuff as the >> locks become NOPs anyway until the MTTCG work is completed. > > From a scalability point of view it would be better to have a single > critical section.
You mean merge the critical region for patching and code-generation? > > From a correctness point of view, we're reading tb->page_addr[1] > without holding a lock. This field is set after qht_insert(tb), > so we might read a yet-uninitialized value. > > I propose to just extend the critical section, like we used to > do with tcg_lock_reset. > > Emilio -- Alex Bennée
