Re: [Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu()
Paolo Bonzini writes: > Signed-off-by: Paolo Bonzini Reviewed-by: Alex Bennée > --- > cpus-common.c | 33 +++-- > include/qom/cpu.h | 14 ++ > 2 files changed, 45 insertions(+), 2 deletions(-) > > diff --git a/cpus-common.c b/cpus-common.c > index 429652c..38b1d55 100644 > --- a/cpus-common.c > +++ b/cpus-common.c > @@ -18,6 +18,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/main-loop.h" > #include "exec/cpu-common.h" > #include "qom/cpu.h" > #include "sysemu/cpus.h" > @@ -106,7 +107,7 @@ struct qemu_work_item { > struct qemu_work_item *next; > run_on_cpu_func func; > void *data; > -bool free, done; > +bool free, exclusive, done; > }; > > static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi) > @@ -139,6 +140,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, > void *data, > wi.data = data; > wi.done = false; > wi.free = false; > +wi.exclusive = false; > > queue_work_on_cpu(cpu, &wi); > while (!atomic_mb_read(&wi.done)) { > @@ -229,6 +231,19 @@ void cpu_exec_end(CPUState *cpu) > qemu_mutex_unlock(&qemu_cpu_list_lock); > } > > +void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data) > +{ > +struct qemu_work_item *wi; > + > +wi = g_malloc0(sizeof(struct qemu_work_item)); > +wi->func = func; > +wi->data = data; > +wi->free = true; > +wi->exclusive = true; > + > +queue_work_on_cpu(cpu, wi); > +} > + > void process_queued_cpu_work(CPUState *cpu) > { > struct qemu_work_item *wi; > @@ -245,7 +260,21 @@ void process_queued_cpu_work(CPUState *cpu) > cpu->queued_work_last = NULL; > } > qemu_mutex_unlock(&cpu->work_mutex); > -wi->func(cpu, wi->data); > +if (wi->exclusive) { > +/* Running work items outside the BQL avoids the following > deadlock: > + * 1) start_exclusive() is called with the BQL taken while > another > + * CPU is running; 2) cpu_exec in the other CPU tries to takes > the > + * BQL, so it goes to sleep; start_exclusive() is sleeping too, > so > + * neither CPU can proceed. > + */ > +qemu_mutex_unlock_iothread(); > +start_exclusive(); > +wi->func(cpu, wi->data); > +end_exclusive(); > +qemu_mutex_lock_iothread(); > +} else { > +wi->func(cpu, wi->data); > +} > qemu_mutex_lock(&cpu->work_mutex); > if (wi->free) { > g_free(wi); > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 934c07a..4092dd9 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -656,6 +656,20 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, > void *data); > void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); > > /** > + * async_safe_run_on_cpu: > + * @cpu: The vCPU to run on. > + * @func: The function to be executed. > + * @data: Data to pass to the function. > + * > + * Schedules the function @func for execution on the vCPU @cpu > asynchronously, > + * while all other vCPUs are sleeping. > + * > + * Unlike run_on_cpu and async_run_on_cpu, the function is run outside the > + * BQL. > + */ > +void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); > + > +/** > * qemu_get_cpu: > * @index: The CPUState@cpu_index value of the CPU to obtain. > * -- Alex Bennée
Re: [Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu()
On 09/23/2016 12:31 AM, Paolo Bonzini wrote: Signed-off-by: Paolo Bonzini --- cpus-common.c | 33 +++-- include/qom/cpu.h | 14 ++ 2 files changed, 45 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
[Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu()
Signed-off-by: Paolo Bonzini --- cpus-common.c | 33 +++-- include/qom/cpu.h | 14 ++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/cpus-common.c b/cpus-common.c index 429652c..38b1d55 100644 --- a/cpus-common.c +++ b/cpus-common.c @@ -18,6 +18,7 @@ */ #include "qemu/osdep.h" +#include "qemu/main-loop.h" #include "exec/cpu-common.h" #include "qom/cpu.h" #include "sysemu/cpus.h" @@ -106,7 +107,7 @@ struct qemu_work_item { struct qemu_work_item *next; run_on_cpu_func func; void *data; -bool free, done; +bool free, exclusive, done; }; static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi) @@ -139,6 +140,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data, wi.data = data; wi.done = false; wi.free = false; +wi.exclusive = false; queue_work_on_cpu(cpu, &wi); while (!atomic_mb_read(&wi.done)) { @@ -229,6 +231,19 @@ void cpu_exec_end(CPUState *cpu) qemu_mutex_unlock(&qemu_cpu_list_lock); } +void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data) +{ +struct qemu_work_item *wi; + +wi = g_malloc0(sizeof(struct qemu_work_item)); +wi->func = func; +wi->data = data; +wi->free = true; +wi->exclusive = true; + +queue_work_on_cpu(cpu, wi); +} + void process_queued_cpu_work(CPUState *cpu) { struct qemu_work_item *wi; @@ -245,7 +260,21 @@ void process_queued_cpu_work(CPUState *cpu) cpu->queued_work_last = NULL; } qemu_mutex_unlock(&cpu->work_mutex); -wi->func(cpu, wi->data); +if (wi->exclusive) { +/* Running work items outside the BQL avoids the following deadlock: + * 1) start_exclusive() is called with the BQL taken while another + * CPU is running; 2) cpu_exec in the other CPU tries to takes the + * BQL, so it goes to sleep; start_exclusive() is sleeping too, so + * neither CPU can proceed. + */ +qemu_mutex_unlock_iothread(); +start_exclusive(); +wi->func(cpu, wi->data); +end_exclusive(); +qemu_mutex_lock_iothread(); +} else { +wi->func(cpu, wi->data); +} qemu_mutex_lock(&cpu->work_mutex); if (wi->free) { g_free(wi); diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 934c07a..4092dd9 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -656,6 +656,20 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); /** + * async_safe_run_on_cpu: + * @cpu: The vCPU to run on. + * @func: The function to be executed. + * @data: Data to pass to the function. + * + * Schedules the function @func for execution on the vCPU @cpu asynchronously, + * while all other vCPUs are sleeping. + * + * Unlike run_on_cpu and async_run_on_cpu, the function is run outside the + * BQL. + */ +void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); + +/** * qemu_get_cpu: * @index: The CPUState@cpu_index value of the CPU to obtain. * -- 2.7.4
Re: [Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu()
On 21/09/2016 18:08, Emilio G. Cota wrote: > On Mon, Sep 19, 2016 at 14:50:57 +0200, Paolo Bonzini wrote: >> We have to run safe work items outside the BQL; for now keep other >> work items within the BQL, though this can be changed relatively >> easily as a follow-up. >> >> Signed-off-by: Paolo Bonzini >> --- >> cpus-common.c | 33 +++-- >> include/qom/cpu.h | 14 ++ >> 2 files changed, 45 insertions(+), 2 deletions(-) >> >> diff --git a/cpus-common.c b/cpus-common.c >> index 6adc982..f7ad534 100644 >> --- a/cpus-common.c >> +++ b/cpus-common.c >> @@ -106,7 +106,7 @@ struct qemu_work_item { >> struct qemu_work_item *next; >> run_on_cpu_func func; >> void *data; >> -bool free, done; >> +bool free, exclusive, done; >> }; >> >> static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi) >> @@ -139,6 +139,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, >> void *data, >> wi.data = data; >> wi.done = false; >> wi.free = false; >> +wi.exclusive = false; >> >> queue_work_on_cpu(cpu, &wi); >> while (!atomic_mb_read(&wi.done)) { >> @@ -157,6 +158,7 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func >> func, void *data) >> wi->func = func; >> wi->data = data; >> wi->free = true; >> +wi->exclusive = false; > > Just a very pedantic nit: in patch 08/19 we don't set wi->done false because > there's a malloc0 right above this. So we might want to do the same here. > > E. > Ok. Paolo
Re: [Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu()
On Mon, Sep 19, 2016 at 14:50:57 +0200, Paolo Bonzini wrote: > We have to run safe work items outside the BQL; for now keep other > work items within the BQL, though this can be changed relatively > easily as a follow-up. > > Signed-off-by: Paolo Bonzini > --- > cpus-common.c | 33 +++-- > include/qom/cpu.h | 14 ++ > 2 files changed, 45 insertions(+), 2 deletions(-) > > diff --git a/cpus-common.c b/cpus-common.c > index 6adc982..f7ad534 100644 > --- a/cpus-common.c > +++ b/cpus-common.c > @@ -106,7 +106,7 @@ struct qemu_work_item { > struct qemu_work_item *next; > run_on_cpu_func func; > void *data; > -bool free, done; > +bool free, exclusive, done; > }; > > static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi) > @@ -139,6 +139,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, > void *data, > wi.data = data; > wi.done = false; > wi.free = false; > +wi.exclusive = false; > > queue_work_on_cpu(cpu, &wi); > while (!atomic_mb_read(&wi.done)) { > @@ -157,6 +158,7 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func > func, void *data) > wi->func = func; > wi->data = data; > wi->free = true; > +wi->exclusive = false; Just a very pedantic nit: in patch 08/19 we don't set wi->done false because there's a malloc0 right above this. So we might want to do the same here. E.
[Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu()
We have to run safe work items outside the BQL; for now keep other work items within the BQL, though this can be changed relatively easily as a follow-up. Signed-off-by: Paolo Bonzini --- cpus-common.c | 33 +++-- include/qom/cpu.h | 14 ++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/cpus-common.c b/cpus-common.c index 6adc982..f7ad534 100644 --- a/cpus-common.c +++ b/cpus-common.c @@ -106,7 +106,7 @@ struct qemu_work_item { struct qemu_work_item *next; run_on_cpu_func func; void *data; -bool free, done; +bool free, exclusive, done; }; static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi) @@ -139,6 +139,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data, wi.data = data; wi.done = false; wi.free = false; +wi.exclusive = false; queue_work_on_cpu(cpu, &wi); while (!atomic_mb_read(&wi.done)) { @@ -157,6 +158,7 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data) wi->func = func; wi->data = data; wi->free = true; +wi->exclusive = false; queue_work_on_cpu(cpu, wi); } @@ -230,6 +232,19 @@ void cpu_exec_end(CPUState *cpu) qemu_mutex_unlock(&qemu_cpu_list_mutex); } +void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data) +{ +struct qemu_work_item *wi; + +wi = g_malloc0(sizeof(struct qemu_work_item)); +wi->func = func; +wi->data = data; +wi->free = true; +wi->exclusive = true; + +queue_work_on_cpu(cpu, wi); +} + void process_queued_cpu_work(CPUState *cpu) { struct qemu_work_item *wi; @@ -246,7 +261,21 @@ void process_queued_cpu_work(CPUState *cpu) cpu->queued_work_last = NULL; } qemu_mutex_unlock(&cpu->work_mutex); -wi->func(cpu, wi->data); +if (wi->exclusive) { + /* Running work items outside the BQL avoids the following deadlock: +* 1) start_exclusive() is called with the BQL taken while another + * CPU is running; 2) cpu_exec in the other CPU tries to takes the + * BQL, so it goes to sleep; start_exclusive() is sleeping too, so + * neither CPU can proceed. + */ +qemu_mutex_unlock_iothread(); +start_exclusive(); +wi->func(cpu, wi->data); +end_exclusive(); +qemu_mutex_lock_iothread(); +} else { +wi->func(cpu, wi->data); +} qemu_mutex_lock(&cpu->work_mutex); if (wi->free) { g_free(wi); diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 934c07a..4092dd9 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -656,6 +656,20 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); /** + * async_safe_run_on_cpu: + * @cpu: The vCPU to run on. + * @func: The function to be executed. + * @data: Data to pass to the function. + * + * Schedules the function @func for execution on the vCPU @cpu asynchronously, + * while all other vCPUs are sleeping. + * + * Unlike run_on_cpu and async_run_on_cpu, the function is run outside the + * BQL. + */ +void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); + +/** * qemu_get_cpu: * @index: The CPUState@cpu_index value of the CPU to obtain. * -- 2.7.4
[Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu()
Signed-off-by: Paolo Bonzini --- cpus-common.c | 25 +++-- include/qom/cpu.h | 11 +++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/cpus-common.c b/cpus-common.c index 12c8e69..50a92dd 100644 --- a/cpus-common.c +++ b/cpus-common.c @@ -106,7 +106,7 @@ struct qemu_work_item { struct qemu_work_item *next; run_on_cpu_func func; void *data; -bool free, done; +bool free, exclusive, done; }; static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi) @@ -139,6 +139,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data, wi.data = data; wi.done = false; wi.free = false; +wi.exclusive = false; queue_work_on_cpu(cpu, &wi); while (!atomic_mb_read(&wi.done)) { @@ -157,6 +158,7 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data) wi->func = func; wi->data = data; wi->free = true; +wi->exclusive = false; queue_work_on_cpu(cpu, wi); } @@ -230,6 +232,19 @@ void cpu_exec_end(CPUState *cpu) qemu_mutex_unlock(&qemu_cpu_list_mutex); } +void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data) +{ +struct qemu_work_item *wi; + +wi = g_malloc0(sizeof(struct qemu_work_item)); +wi->func = func; +wi->data = data; +wi->free = true; +wi->exclusive = true; + +queue_work_on_cpu(cpu, wi); +} + void process_queued_cpu_work(CPUState *cpu) { struct qemu_work_item *wi; @@ -246,7 +261,13 @@ void process_queued_cpu_work(CPUState *cpu) cpu->queued_work_last = NULL; } qemu_mutex_unlock(&cpu->work_mutex); -wi->func(cpu, wi->data); +if (wi->exclusive) { +start_exclusive(); +wi->func(cpu, wi->data); +end_exclusive(); +} else { +wi->func(cpu, wi->data); +} qemu_mutex_lock(&cpu->work_mutex); if (wi->free) { g_free(wi); diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 934c07a..d1ca31c 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -656,6 +656,17 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); /** + * async_safe_run_on_cpu: + * @cpu: The vCPU to run on. + * @func: The function to be executed. + * @data: Data to pass to the function. + * + * Schedules the function @func for execution on the vCPU @cpu asynchronously, + * while all other vCPUs are sleeping. + */ +void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data); + +/** * qemu_get_cpu: * @index: The CPUState@cpu_index value of the CPU to obtain. * -- 2.7.4