Re: [PATCH 09/10] system/cpus: add bql-watchdog-abort

[Fabiano Rosas <farosas@suse.de>]

"Denis V. Lunev" <den@openvz.org> writes:

> From: Denis V. Lunev <den@openvz.org>
>
> The report says where each thread was when the watchdog looked, not
> how it got there. A core carries every thread's whole stack, at the
> price of the guest. A qemu whose BQL has been held this long is not
> running its vCPUs anyway, so on a host where the answer is worth more
> than the guest, abort after the report.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> Cc: Zhao Liu <zhao1.liu@intel.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  hw/core/machine.c     | 17 +++++++++++++++++
>  include/system/cpus.h |  4 ++++
>  qemu-options.hx       |  5 +++++
>  system/cpus.c         | 19 +++++++++++++++++++
>  4 files changed, 45 insertions(+)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index b55af89516..2abb9e6295 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -403,6 +403,17 @@ static void machine_set_bql_watchdog_ms(Object *obj, Visitor *v,
>      bql_watchdog_set_timeout_ms(value, errp);
>  }
>  
> +static bool machine_get_bql_watchdog_abort(Object *obj, Error **errp)
> +{
> +    return bql_watchdog_get_abort();
> +}
> +
> +static void machine_set_bql_watchdog_abort(Object *obj, bool value,
> +                                           Error **errp)
> +{
> +    bql_watchdog_set_abort(value);
> +}
> +
>  static bool machine_get_new_accel_vmfd_on_reset(Object *obj, Error **errp)
>  {
>      MachineState *ms = MACHINE(obj);
> @@ -1174,6 +1185,12 @@ static void machine_class_init(ObjectClass *oc, const void *data)
>          "Watchdog deadline for a Big QEMU Lock hold, in ms. "
>          "Default: 0 (disabled)");
>  
> +    object_class_property_add_bool(oc, "bql-watchdog-abort",
> +        machine_get_bql_watchdog_abort, machine_set_bql_watchdog_abort);
> +    object_class_property_set_description(oc, "bql-watchdog-abort",
> +        "Abort after the bql-watchdog-ms report, to leave a core dump. "
> +        "Default: off");
> +
>      object_class_property_add_bool(oc, "x-change-vmfd-on-reset",
>          machine_get_new_accel_vmfd_on_reset,
>          machine_set_new_accel_vmfd_on_reset);
> diff --git a/include/system/cpus.h b/include/system/cpus.h
> index 0e1e63bd85..14dbc8ff0d 100644
> --- a/include/system/cpus.h
> +++ b/include/system/cpus.h
> @@ -44,4 +44,8 @@ void cpu_synchronize_all_pre_loadvm(void);
>  bool bql_watchdog_set_timeout_ms(uint64_t ms, Error **errp);
>  uint64_t bql_watchdog_get_timeout_ms(void);
>  
> +/* Abort after reporting, to leave a core behind. */
> +void bql_watchdog_set_abort(bool enable);
> +bool bql_watchdog_get_abort(void);
> +
>  #endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9600620d34..f43d8ca480 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -33,6 +33,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>      "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
>      "                mem-merge=on|off controls memory merge support (default: on)\n"
>      "                bql-watchdog-ms=ms report a Big QEMU Lock held longer than ms (default: 0, off)\n"
> +    "                bql-watchdog-abort=on|off abort after the bql-watchdog-ms report (default: off)\n"
>      "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
>      "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
>      "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
> @@ -94,6 +95,10 @@ SRST
>          milliseconds, up to a minute. The default is 0, which disables the
>          watchdog.
>  
> +    ``bql-watchdog-abort=on|off``
> +        Abort after the ``bql-watchdog-ms`` report, to leave a core dump.
> +        The default is off.
> +
>      ``aes-key-wrap=on|off``
>          Enables or disables AES key wrapping support on s390-ccw hosts.
>          This feature controls whether AES wrapping keys will be created
> diff --git a/system/cpus.c b/system/cpus.c
> index 03a07e4824..a200a07707 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -398,6 +398,7 @@ static int bql_this_tid(void)
>  static QemuThread bql_watchdog_thread;
>  static QemuSemaphore bql_watchdog_sem;
>  static uint64_t bql_watchdog_timeout_ms;
> +static bool bql_watchdog_abort;
>  
>  static void bql_report_thread_states(void)
>  {
> @@ -411,6 +412,16 @@ static void bql_report_thread_states(void)
>      qemu_funlockfile(stderr);
>  }
>  
> +bool bql_watchdog_get_abort(void)
> +{
> +    return qatomic_read(&bql_watchdog_abort);
> +}
> +
> +void bql_watchdog_set_abort(bool enable)
> +{
> +    qatomic_set(&bql_watchdog_abort, enable);
> +}
> +
>  static void *bql_watchdog_fn(void *opaque)
>  {
>      unsigned int last = qatomic_read(&bql_seq);
> @@ -456,6 +467,14 @@ static void *bql_watchdog_fn(void *opaque)
>                       when ?: "", when ? " " : "", timeout_ms,
>                       qatomic_read(&bql_owner_tid));
>          bql_report_thread_states();
> +        if (!qatomic_read(&bql_watchdog_abort)) {
> +            continue;
> +        }
> +        /*
> +         * A core carries the whole of every stack, and a qemu whose BQL has
> +         * been held this long has stopped running its vCPUs anyway.
> +         */
> +        abort();
>      }
>  }

Reviewed-by: Fabiano Rosas <farosas@suse.de>