Re: [PATCH AUTOSEL 6.6 21/21] arm64/sve: Lower the maximum allocation for the SVE ptrace regset

Doug Anderson
2024-02-29T16:51:27Z

Hi,

On Thu, Feb 29, 2024 at 7:50 AM Sasha Levin <sashal@kernel.org> wrote:
>
> From: Mark Brown <broonie@kernel.org>
>
> [ Upstream commit 2813926261e436d33bc74486b51cce60b76edf78 ]
>
> Doug Anderson observed that ChromeOS crashes are being reported which
> include failing allocations of order 7 during core dumps due to ptrace
> allocating storage for regsets:
>
> chrome: page allocation failure: order:7,
> mode:0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO),
> nodemask=(null),cpuset=urgent,mems_allowed=0
> ...
> regset_get_alloc+0x1c/0x28
> elf_core_dump+0x3d8/0xd8c
> do_coredump+0xeb8/0x1378
>
> with further investigation showing that this is:
>
> [ 66.957385] DOUG: Allocating 279584 bytes
>
> which is the maximum size of the SVE regset. As Doug observes it is not
> entirely surprising that such a large allocation of contiguous memory might
> fail on a long running system.
>
> The SVE regset is currently sized to hold SVE registers with a VQ of
> SVE_VQ_MAX which is 512, substantially more than the architectural maximum
> of 16 which we might see even in a system emulating the limits of the
> architecture. Since we don't expose the size we tell the regset core
> externally let's define ARCH_SVE_VQ_MAX with the actual architectural
> maximum and use that for the regset, we'll still overallocate most of the
> time but much less so which will be helpful even if the core is fixed to
> not require contiguous allocations.
>
> Specify ARCH_SVE_VQ_MAX in terms of the maximum value that can be written
> into ZCR_ELx.LEN (where this is set in the hardware). For consistency
> update the maximum SME vector length to be specified in the same style
> while we are at it.
>
> We could also teach the ptrace core about runtime discoverable regset sizes
> but that would be a more invasive change and this is being observed in
> practical systems.
>
> Reported-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Tested-by: Douglas Anderson <dianders@chromium.org>
> Link: https://lore.kernel.org/r/20240213-arm64-sve-ptrace-regset-size-v2-1-c7600ca74b9b@kernel.org
> Signed-off-by: Will Deacon <will@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> arch/arm64/include/asm/fpsimd.h | 12 ++++++------
> arch/arm64/kernel/ptrace.c | 3 ++-
> 2 files changed, 8 insertions(+), 7 deletions(-)

As I mentioned [1], there's a hidden dependency here and without it
the patch doesn't actually do anything useful in kernel 6.6 nor kernel
6.1. Maybe the right answer is to backport this with the hardcoded
value of "16" for those older kernels? Maybe Mark has a better
suggestion?

[1] https://lore.kernel.org/r/CAD=FV=WSi=9V-Oe5eq0J-Uew45cX9JfgB8me-Nw-iFRfXm59Xg@mail.gmail.com