[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 4/8] linux-user: Rework exclusive operation mechan
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [RFC 4/8] linux-user: Rework exclusive operation mechanism |
Date: |
Mon, 27 Jun 2016 10:02:22 +0100 |
User-agent: |
mu4e 0.9.17; emacs 25.0.95.5 |
Sergey Fedorov <address@hidden> writes:
> From: Sergey Fedorov <address@hidden>
>
> A single variable 'pending_cpus' was used for both counting currently
> running CPUs and for signalling the pending exclusive operation request.
>
> To prepare for supporting operations which requires a quiescent state,
> like translation buffer flush, it is useful to keep a counter of
> currently running CPUs always up to date.
>
> Use a separate variable 'tcg_pending_cpus' to count for currently
> running CPUs and a separate variable 'exclusive_pending' to indicate
> that there's an exclusive operation pending.
>
> Signed-off-by: Sergey Fedorov <address@hidden>
> Signed-off-by: Sergey Fedorov <address@hidden>
> ---
> linux-user/main.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index b9a4e0ea45ac..485336f78b8f 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -111,7 +111,8 @@ static pthread_mutex_t cpu_list_mutex =
> PTHREAD_MUTEX_INITIALIZER;
> static pthread_mutex_t exclusive_lock = PTHREAD_MUTEX_INITIALIZER;
> static pthread_cond_t exclusive_cond = PTHREAD_COND_INITIALIZER;
> static pthread_cond_t exclusive_resume = PTHREAD_COND_INITIALIZER;
> -static int pending_cpus;
> +static bool exclusive_pending;
> +static int tcg_pending_cpus;
I'm not sure you need to re-name to tcg_pending_cpus as TCG is implied
for linux-user. Also they are not really CPUs (although we are using the
CPU structure for each running thread). I'm not sure if there is a
neater way to make the distinction clear.
>
> /* Make sure everything is in a consistent state for calling fork(). */
> void fork_start(void)
> @@ -133,7 +134,8 @@ void fork_end(int child)
> QTAILQ_REMOVE(&cpus, cpu, node);
> }
> }
> - pending_cpus = 0;
> + tcg_pending_cpus = 0;
> + exclusive_pending = false;
> pthread_mutex_init(&exclusive_lock, NULL);
> pthread_mutex_init(&cpu_list_mutex, NULL);
> pthread_cond_init(&exclusive_cond, NULL);
> @@ -150,7 +152,7 @@ void fork_end(int child)
> must be held. */
> static inline void exclusive_idle(void)
> {
> - while (pending_cpus) {
> + while (exclusive_pending) {
> pthread_cond_wait(&exclusive_resume, &exclusive_lock);
> }
> }
> @@ -164,15 +166,14 @@ static inline void start_exclusive(void)
> pthread_mutex_lock(&exclusive_lock);
> exclusive_idle();
>
> - pending_cpus = 1;
> + exclusive_pending = true;
> /* Make all other cpus stop executing. */
> CPU_FOREACH(other_cpu) {
> if (other_cpu->running) {
> - pending_cpus++;
> cpu_exit(other_cpu);
> }
> }
> - if (pending_cpus > 1) {
> + while (tcg_pending_cpus) {
> pthread_cond_wait(&exclusive_cond, &exclusive_lock);
> }
> }
> @@ -180,7 +181,7 @@ static inline void start_exclusive(void)
> /* Finish an exclusive operation. */
> static inline void __attribute__((unused)) end_exclusive(void)
> {
> - pending_cpus = 0;
> + exclusive_pending = false;
> pthread_cond_broadcast(&exclusive_resume);
> pthread_mutex_unlock(&exclusive_lock);
> }
> @@ -191,6 +192,7 @@ static inline void cpu_exec_start(CPUState *cpu)
> pthread_mutex_lock(&exclusive_lock);
> exclusive_idle();
> cpu->running = true;
> + tcg_pending_cpus++;
These aren't TLS variables so shouldn't we be ensuring all access is atomic?
> pthread_mutex_unlock(&exclusive_lock);
> }
>
> @@ -199,11 +201,9 @@ static inline void cpu_exec_end(CPUState *cpu)
> {
> pthread_mutex_lock(&exclusive_lock);
> cpu->running = false;
> - if (pending_cpus > 1) {
> - pending_cpus--;
> - if (pending_cpus == 1) {
> - pthread_cond_signal(&exclusive_cond);
> - }
> + tcg_pending_cpus--;
> + if (!tcg_pending_cpus) {
> + pthread_cond_broadcast(&exclusive_cond);
> }
Couldn't two threads race to -1 here?
> exclusive_idle();
> pthread_mutex_unlock(&exclusive_lock);
--
Alex Bennée
- [Qemu-devel] [RFC 0/8] cpu-exec: Safe work in quiescent state, Sergey Fedorov, 2016/06/19
- [Qemu-devel] [RFC 4/8] linux-user: Rework exclusive operation mechanism, Sergey Fedorov, 2016/06/19
- Re: [Qemu-devel] [RFC 4/8] linux-user: Rework exclusive operation mechanism,
Alex Bennée <=
- [Qemu-devel] [RFC 5/8] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick(), Sergey Fedorov, 2016/06/19
- [Qemu-devel] [RFC 2/8] cpus: Move common code out of {async_, }run_on_cpu(), Sergey Fedorov, 2016/06/19
- [Qemu-devel] [RFC 1/8] cpus: pass CPUState to run_on_cpu helpers, Sergey Fedorov, 2016/06/19
- [Qemu-devel] [RFC 3/8] cpus: Add 'qemu_work_cond' usage wrappers, Sergey Fedorov, 2016/06/19
- [Qemu-devel] [RFC 7/8] cpu-exec-common: Introduce async_safe_run_on_cpu(), Sergey Fedorov, 2016/06/19