Am 09.04.24 um 14:48 schrieb Friedrich Weber:
> On processors with the `split_lock_detect` feature, the kernel will
> detect user-space split locks and, by default, apply a "misery mode"
> to offending tasks [1]. When a split lock is detected, the misery mode
> artificially slows down the task by forcing a 10ms and serialization
> with other offending tasks. This can also apply to vCPU threads of VM
> guests that perform unaligned memory access. In this case, the misery
> mode can result in temporary vCPU freezes of 10ms or more.
>
> Currently, correlating observed vCPU freezes with split lock detection
> is hard for users: Split lock detection does log a warning, but the
> warning does not mention the slowdown, and is logged only once for
> each task, which makes troubleshooting hard in case of long-running
> VMs. Currently, vCPU threads of OVMF VMs seem to already produce one
> such warning on boot, which masks any split locks taken later by the
> guest OS.
>
> To ease troubleshooting, add a patch that warns more prominently about
> misery mode. With this patch, the kernel warns every time a task is
> artificially slowed down. Even though the warnings are rate-limited by
> the `printk_ratelimit` mechanism, there is a risk of large volume of
> warnings, but this seems preferable to the misery mode going
> unnoticed.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/x86/kernel/cpu/intel.c?id=727209376f49
>
> Signed-off-by: Friedrich Weber <f.we...@proxmox.com>
> ---
>
> Notes:
> With this patch applied, I see a risk that some users will get a *lot*
> of warnings about misery mode. By default, these are rate-limited to
> 10 messages per 5 seconds, but depending on the workload, this can
> still mean a lot of messages. This is good for driving user
> attention towards the split locks, but still, I wonder whether this
> might spam the journal too much. What do you think?
>
I know this is an upper limit, but 10 messages per 5 seconds is too much
IMHO. In particular, because certain guest OSes cannot be changed to
avoid taking split locks and users will get scared by these messages.
Unfortunately, reported_split_lock is just a flag and cannot be used as
a counter to log every N taken split locks, which for the right N might
be closer to what we/users actually want. Turning the flag into a
counter would mean wasting more space in the "current" struct, not sure
if that's worth it.
> Also, currently there is no option to stop the warnings without
> disabling misery mode (or split lock detection in general). Should
> there be such an option? I could imagine making the
> `split_lock_mitigate` sysctl ternary:
>
> 2: Enable misery mode and warn every time (new behavior, new default)
> 1: Enable misery mode but warn only once (old behavior)
> 0: Disable misery mode and warn only once
>
> ... or even completely decouple misery mode and warning behavior?
> What do you think?
>
If we go for the current patch, I'm in favor of having a switch to turn
the frequent warnings off. I feel like it should be a separate switch then.
> ...lways-warn-when-applying-misery-mode.patch | 67 +++++++++++++++++++
> 1 file changed, 67 insertions(+)
> create mode 100644
> patches/kernel/0012-x86-split_lock-always-warn-when-applying-misery-mode.patch
>
> diff --git
> a/patches/kernel/0012-x86-split_lock-always-warn-when-applying-misery-mode.patch
>
> b/patches/kernel/0012-x86-split_lock-always-warn-when-applying-misery-mode.patch
> new file mode 100644
> index 0000000..658de5e
> --- /dev/null
> +++
> b/patches/kernel/0012-x86-split_lock-always-warn-when-applying-misery-mode.patch
> @@ -0,0 +1,67 @@
> +From f8db8223735c529a1dcaaf41217024ee4091a464 Mon Sep 17 00:00:00 2001
> +From: Friedrich Weber <f.we...@proxmox.com>
> +Date: Tue, 9 Apr 2024 10:57:43 +0200
> +Subject: [PATCH] x86/split_lock: always warn when applying misery to task
> +
> +Since commits b041b525dab9 ("x86/split_lock: Make life miserable for
> +split lockers") and 727209376f49 ("x86/split_lock: Add sysctl to
> +control the misery mode"), split lock detection artificially slows
> +down the offending task ("misery mode") if the `split_lock_mitigate`
> +sysctl is enabled (which it is by default). The corresponding warning
> +does not mention the slowdown, and is logged only once for each task.
> +In case of long-running VMs, this makes it hard to correlate observed
> +vCPU freezes to the split lock detection misery mode.
> +
A start would be to extend the warning to mention that it won't be
logged again and there can be slowdowns.
Maybe you can also ask upstream for feedback and suggestions.
> +To make troubleshooting easier, change the warning behavior:
> +
> +- If the `split_lock_mitigate` sysctl is enabled, warn every time
> + misery is applied to a task, and rephrase the warning. Even though
> + the warnings are rate-limited by the `printk_ratelimit` mechanism,
> + this may result in a large volume of warnings for split-lock-heavy
> + workloads, but this seems preferable to the misery mode going
> + unnoticed.
> +- If the `split_lock_mitigate` sysctl is disabled, warn only once (as
> + before), but mention explicitly that no further action is taken and
> + subsequent traps will not be logged.
> +
> +Signed-off-by: Friedrich Weber <f.we...@proxmox.com>
As for the patch itself:
Reviewed-by: Fiona Ebner <f.eb...@proxmox.com>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
