On 03/06/16 23:40, Alex Bennée wrote: > diff --git a/translate-all.c b/translate-all.c > index ec6fdbb..e3f44d9 100644 > --- a/translate-all.c > +++ b/translate-all.c (snip) > @@ -60,6 +61,7 @@ > > /* #define DEBUG_TB_INVALIDATE */ > /* #define DEBUG_TB_FLUSH */ > +/* #define DEBUG_LOCKING */
A bit of bikeshedding: have you considered naming it 'DEBUG_LOCKS'. How does it sound for a native English speaker? :) > /* make various TB consistency checks */ > /* #define DEBUG_TB_CHECK */ > > @@ -68,6 +70,28 @@ > #undef DEBUG_TB_CHECK > #endif > > +/* Access to the various translations structures need to be serialised via > locks > + * for consistency. This is automatic for SoftMMU based system > + * emulation due to its single threaded nature. In user-mode emulation > + * access to the memory related structures are protected with the > + * mmap_lock. > + */ > +#ifdef DEBUG_LOCKING > +#define DEBUG_MEM_LOCKS 1 > +#else > +#define DEBUG_MEM_LOCKS 0 > +#endif > + > +#ifdef CONFIG_SOFTMMU > +#define assert_memory_lock() do { /* nothing */ } while (0) > +#else > +#define assert_memory_lock() do { \ > + if (DEBUG_MEM_LOCKS) { \ > + g_assert(have_mmap_lock()); \ > + } \ > + } while (0) > +#endif > + Why not simply: #if !defined(DEBUG_LOCKING) || defined(CONFIG_SOFTMMU) # define assert_memory_lock() do { /* nothing */ } while (0) #else # define assert_memory_lock() g_assert(have_mmap_lock()) #endif One more nit: maybe it would be a bit more clear to name it after the lock name, i.e. assert_mmap_lock(), or check_mmap_lock(), or debug_mmap_lock() etc? Thanks, Sergey