Received-SPF: pass (google.com: domain of linux-kernel+bounces-82459-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240205092626.v2.1.Id9ad163b60d21c9e56c2d686b0cc9083a8ba7924@changeid>
In-Reply-To: <20240205092626.v2.1.Id9ad163b60d21c9e56c2d686b0cc9083a8ba7924@changeid>
From: Doug Anderson <dianders@chromium.org>
Date: Mon, 26 Feb 2024 15:55:28 -0800
Message-ID: <CAD=FV=WgGuJLBWmXBOU5oHMvWP2M1cSMS201K8HpyXSYiBPJXQ@mail.gmail.com>
Subject: Re: [PATCH v2] regset: use kvzalloc() for regset_get_alloc()
To: Alexander Viro <viro@zeniv.linux.org.uk>, Christian Brauner <brauner@kernel.org>
Cc: Mark Brown <broonie@kernel.org>, Catalin Marinas <catalin.marinas@arm.com>, 
	Will Deacon <will@kernel.org>, Dave Martin <Dave.Martin@arm.com>, Oleg Nesterov <oleg@redhat.com>, 
	linux-arm-kernel@lists.infradead.org, Matthew Wilcox <willy@infradead.org>, 
	Eric Biederman <ebiederm@xmission.com>, Jan Kara <jack@suse.cz>, Kees Cook <keescook@chromium.org>, 
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Mon, Feb 5, 2024 at 9:27=E2=80=AFAM Douglas Anderson <dianders@chromium.=
org> wrote:
>
> While browsing through ChromeOS crash reports, I found one with an
> allocation failure that looked like this:
>
>   chrome: page allocation failure: order:7,
>           mode:0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO),
>           nodemask=3D(null),cpuset=3Durgent,mems_allowed=3D0
>   CPU: 7 PID: 3295 Comm: chrome Not tainted
>           5.15.133-20574-g8044615ac35c #1 (HASH:1162 1)
>   Hardware name: Google Lazor (rev3 - 8) with KB Backlight (DT)
>   Call trace:
>   ...
>   warn_alloc+0x104/0x174
>   __alloc_pages+0x5f0/0x6e4
>   kmalloc_order+0x44/0x98
>   kmalloc_order_trace+0x34/0x124
>   __kmalloc+0x228/0x36c
>   __regset_get+0x68/0xcc
>   regset_get_alloc+0x1c/0x28
>   elf_core_dump+0x3d8/0xd8c
>   do_coredump+0xeb8/0x1378
>   get_signal+0x14c/0x804
>   ...
>
> An order 7 allocation is (1 << 7) contiguous pages, or 512K. It's not
> a surprise that this allocation failed on a system that's been running
> for a while.
>
> More digging showed that it was fairly easy to see the order 7
> allocation by just sending a SIGQUIT to chrome (or other processes) to
> generate a core dump. The actual amount being allocated was 279,584
> bytes and it was for "core_note_type" NT_ARM_SVE.
>
> There was quite a bit of discussion [1] on the mailing lists in
> response to my v1 patch attempting to switch to vmalloc. The overall
> conclusion was that we could likely reduce the 279,584 byte allocation
> by quite a bit and Mark Brown has sent a patch to that effect [2].
> However even with the 279,584 byte allocation gone there are still
> 65,552 byte allocations. These are just barely more than the 65,536
> bytes and thus would require an order 5 allocation.
>
> An order 5 allocation is still something to avoid unless necessary and
> nothing needs the memory here to be contiguous. Change the allocation
> to kvzalloc() which should still be efficient for small allocations
> but doesn't force the memory subsystem to work hard (and maybe fail)
> at getting a large contiguous chunk.
>
> [1] https://lore.kernel.org/r/20240201171159.1.Id9ad163b60d21c9e56c2d686b=
0cc9083a8ba7924@changeid
> [2] https://lore.kernel.org/r/20240203-arm64-sve-ptrace-regset-size-v1-1-=
2c3ba1386b9e@kernel.org
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - Use kvzalloc() instead of vmalloc().
> - Update description based on v1 discussion.
>
>  fs/binfmt_elf.c | 2 +-
>  kernel/regset.c | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)

Just wanted to check in to see if there's anything else that I need to
do here. Mark's patch to avoid the order 7 allocations [1] has landed,
but we still want this kvzalloc() because the order 5 allocations
can't really be avoided. I'm happy to sit tight for longer but just
wanted to make sure it was clear that we still want my patch _in
addition_ to Mark's patch and to see if there was anything else you
needed me to do.

Thanks!

[1] https://lore.kernel.org/r/20240213-arm64-sve-ptrace-regset-size-v2-1-c7=
600ca74b9b@kernel.org