Re: [Qemu-devel] [PATCH] linux-user: always start with parallel_cpus set to true

["Alex Bennée" <alex.bennee@linaro.org>]

Laurent Vivier <laurent@vivier.eu> writes:

> We always need real atomics, as we can have shared memory between
> processes.
>
> A good test case is the example from futex(2), futex_demo.c:
>
> the use case is
>
>     mmap(...);
>     fork();
>
>     Parent and Child:
>
>     while(...)
>         __sync_bool_compare_and_swap(...)
>         ...
>         futex(...)
>
> In this case we need real atomics in __sync_bool_compare_and_swap(),
> but as parallel_cpus is set to 0, we don't have.
>
> We also revert "b67cb68 linux-user: enable parallel code generation on clone"
> as parallel_cpus in unconditionally set now.
>
> Of course, this doesn't fix atomics that are emulated using
> cpu_loop_exit_atomic() as we can't stop virtual CPUs from another
> processes.

This seems a bit of a hit for something that might never get called.
Could we not move b67cb68 out of the thread fork leg and do it for any
fork? After all the tb_flush will ensure that all code by both parent
and child will be using full atomics at that point?

>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/syscall.c | 8 --------
>  translate-all.c      | 4 ++++
>  2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 7b77503..db697c0 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -6164,14 +6164,6 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
>          sigfillset(&sigmask);
>          sigprocmask(SIG_BLOCK, &sigmask, &info.sigmask);
>
> -        /* If this is our first additional thread, we need to ensure we
> -         * generate code for parallel execution and flush old translations.
> -         */
> -        if (!parallel_cpus) {
> -            parallel_cpus = true;
> -            tb_flush(cpu);
> -        }
> -
>          ret = pthread_create(&info.thread, &attr, clone_func, &info);
>          /* TODO: Free new CPU state if thread creation failed.  */
>
> diff --git a/translate-all.c b/translate-all.c
> index 3dd9214..0b0bb09 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -142,7 +142,11 @@ static void *l1_map[V_L1_MAX_SIZE];
>
>  /* code generation context */
>  TCGContext tcg_ctx;
> +#ifdef CONFIG_USER_ONLY
> +bool parallel_cpus = true;
> +#else
>  bool parallel_cpus;
> +#endif
>
>  /* translation block context */
>  #ifdef CONFIG_USER_ONLY


--
Alex Bennée