Re: [PATCH] regset: use vmalloc() for regset_get_alloc()

From: Mark Brown <broonie@kernel.org>
To: Dave Martin <Dave.Martin@arm.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Doug Anderson <dianders@chromium.org>,
	Christian Brauner <brauner@kernel.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, Oleg Nesterov <oleg@redhat.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] regset: use vmalloc() for regset_get_alloc()
Date: Fri, 2 Feb 2024 19:42:58 +0000	[thread overview]
Message-ID: <d7154f86-d185-495d-aa84-63d4561f1e47@sirena.org.uk> (raw)
In-Reply-To: <Zb0vem7KC28gmT5U@e133380.arm.com>

[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]

On Fri, Feb 02, 2024 at 06:07:54PM +0000, Dave Martin wrote:

> So, if the only reason for trying to migrate to vmalloc() is to cope
> with an insanely sized regset on arm64, I think somehow or other we can
> avoid that.

With SME we do routinely see the full glory of the 64K regset for ZA in
emulated systems so I think we have to treat it as an issue.

> Options:

>  a) bring back ->get_size() so that we can allocate the correct size
> before generating the regset data;

>  b) make aarch64_regsets[] __ro_after_init and set
> aarch64_regsets[REGSET_SVE].n based on the boot-time probed maximum size
> (which will be sane); or

Either of those seems sensible to me, a function would minimise the size
of allocations based on the process configuration which would be nice
and given that we're doing allocations it's probably reasonable
overhead.

>  c) allow membufs to grow if needed (sounds fragile though, and may be
> hard to justify just for one arch?).

I'm having a hard time getting enthusiastic about that one for the
reasons you mention.

We can also just lower the maximum size we tell the ptrace core to the
actual architectural maximum since AFAICT we don't expose that anywhere
external, I've got a patch in CI for that.  We'd still be allocating
more memory than we need for practical systems but less extravagantly
so.  It seems more suitable for an immediate fix for people to pick up
for production.

It did occur to me at some point in the past that we should avoid
telling the core about regsets that aren't physically supported on the
current system, I didn't get round to looking at that yet.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]