| Message ID | 20220617144857.34189-3-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | CPU: Detect put cpu register errors for migrations | expand |
On Fri, 17 Jun 2022 at 15:57, Peter Xu <peterx@redhat.com> wrote: > > This version of run_on_cpu() allows to take an Error** to detect errors. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > cpus-common.c | 27 +++++++++++++++++++++++++++ > include/hw/core/cpu.h | 26 ++++++++++++++++++++++++++ > softmmu/cpus.c | 6 ++++++ > 3 files changed, 59 insertions(+) > +/** > + * run_on_cpu2: > + * @cpu: The vCPU to run on. > + * @func: The function to be executed. > + * @data: Data to pass to the function. > + * @errp: The Error** pointer to be passed into @func2. > + * > + * Schedules the function @func2 for execution on the vCPU @cpu, capture > + * any error and put it into *@errp when provided. > + */ > +void run_on_cpu2(CPUState *cpu, run_on_cpu_func2 func2, run_on_cpu_data data, > + Error **errp); Can you give this a more descriptive name, please, unless the plan is to convert all the users of the existing run_on_cpu() and then rename this function to that ? thanks -- PMM
On Thu, Jun 23, 2022 at 02:04:03PM +0100, Peter Maydell wrote: > On Fri, 17 Jun 2022 at 15:57, Peter Xu <peterx@redhat.com> wrote: > > > > This version of run_on_cpu() allows to take an Error** to detect errors. > > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > --- > > cpus-common.c | 27 +++++++++++++++++++++++++++ > > include/hw/core/cpu.h | 26 ++++++++++++++++++++++++++ > > softmmu/cpus.c | 6 ++++++ > > 3 files changed, 59 insertions(+) > > > +/** > > + * run_on_cpu2: > > + * @cpu: The vCPU to run on. > > + * @func: The function to be executed. > > + * @data: Data to pass to the function. > > + * @errp: The Error** pointer to be passed into @func2. > > + * > > + * Schedules the function @func2 for execution on the vCPU @cpu, capture > > + * any error and put it into *@errp when provided. > > + */ > > +void run_on_cpu2(CPUState *cpu, run_on_cpu_func2 func2, run_on_cpu_data data, > > + Error **errp); > > Can you give this a more descriptive name, please, Ack on the rename, though do you have a suggestion? I did thought about things like run_on_cpu_with_error_captured but that's awfully long.. The use of "suffix 2" seem to be an option in this case and there're users of it even on published KVM interfaces (to name some, KVM_[SET|GET]_PIT2, KVM_[SET|GET]_CPUID2, KVM_[SET|GET]_SREGS2, KVM_GET_XSAVE2..), while this is qemu helper so we can even rename when we want. > unless the plan is to convert all the users of the existing run_on_cpu() > and then rename this function to that ? No plan for that, since I don't see a strong need and maybe many callers do not care about retval. What I plan to do is to fix the immediate migration issue only so we at least don't hit hard-to-debug bugs even when migration completed succeeded (it seems) on QEMU level. That's also why I used a separate helper just to keep the rest untouched. We could move to the new one in other codes when proper, but that's not part of the goal of this series. Thanks,
diff --git a/cpus-common.c b/cpus-common.c index 1db7bbbb88..1d67c0c655 100644 --- a/cpus-common.c +++ b/cpus-common.c @@ -167,6 +167,33 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data, } } +void do_run_on_cpu2(CPUState *cpu, run_on_cpu_func2 func2, run_on_cpu_data data, + QemuMutex *mutex, Error **errp) +{ + struct qemu_work_item wi; + + if (qemu_cpu_is_self(cpu)) { + func2(cpu, data, errp); + return; + } + + wi.func2 = func2; + wi.data = data; + wi.done = false; + wi.free = false; + wi.exclusive = false; + wi.has_errp = true; + wi.errp = errp; + + queue_work_on_cpu(cpu, &wi); + while (!qatomic_mb_read(&wi.done)) { + CPUState *self_cpu = current_cpu; + + qemu_cond_wait(&qemu_work_cond, mutex); + current_cpu = self_cpu; + } +} + void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data) { struct qemu_work_item *wi; diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 7a303576d0..4bb40a03cf 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -709,6 +709,19 @@ bool cpu_is_stopped(CPUState *cpu); void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data, QemuMutex *mutex); +/** + * do_run_on_cpu2: + * @cpu: The vCPU to run on. + * @func2: The function to be executed. + * @data: Data to pass to the function. + * @mutex: Mutex to release while waiting for @func2 to run. + * @errp: The Error** pointer to be passed into @func2. + * + * Used internally in the implementation of run_on_cpu2. + */ +void do_run_on_cpu2(CPUState *cpu, run_on_cpu_func2 func2, run_on_cpu_data data, + QemuMutex *mutex, Error **errp); + /** * run_on_cpu: * @cpu: The vCPU to run on. @@ -719,6 +732,19 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data, */ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data); +/** + * run_on_cpu2: + * @cpu: The vCPU to run on. + * @func: The function to be executed. + * @data: Data to pass to the function. + * @errp: The Error** pointer to be passed into @func2. + * + * Schedules the function @func2 for execution on the vCPU @cpu, capture + * any error and put it into *@errp when provided. + */ +void run_on_cpu2(CPUState *cpu, run_on_cpu_func2 func2, run_on_cpu_data data, + Error **errp); + /** * async_run_on_cpu: * @cpu: The vCPU to run on. diff --git a/softmmu/cpus.c b/softmmu/cpus.c index 23b30484b2..898363a1d0 100644 --- a/softmmu/cpus.c +++ b/softmmu/cpus.c @@ -391,6 +391,12 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data) do_run_on_cpu(cpu, func, data, &qemu_global_mutex); } +void run_on_cpu2(CPUState *cpu, run_on_cpu_func2 func2, run_on_cpu_data data, + Error **errp) +{ + do_run_on_cpu2(cpu, func2, data, &qemu_global_mutex, errp); +} + static void qemu_cpu_stop(CPUState *cpu, bool exit) { g_assert(qemu_cpu_is_self(cpu));
This version of run_on_cpu() allows to take an Error** to detect errors. Signed-off-by: Peter Xu <peterx@redhat.com> --- cpus-common.c | 27 +++++++++++++++++++++++++++ include/hw/core/cpu.h | 26 ++++++++++++++++++++++++++ softmmu/cpus.c | 6 ++++++ 3 files changed, 59 insertions(+)