Re: [PATCH] arm64/sve: Document that __SVE_VQ_MAX is much larger than needed

From: Mark Brown
Date: Wed Feb 07 2024 - 07:49:18 EST


On Wed, Feb 07, 2024 at 12:01:43PM +0000, Dave Martin wrote:
> On Tue, Feb 06, 2024 at 04:27:01PM +0000, Mark Brown wrote:

> > +/*
> > + * Note that for future proofing __SVE_VQ_MAX is defined much larger
> > + * than the actual architecture maximum of 16.
> > + */

> I think that putting shadow #defines in comments in UAPI headers is a
> really bad idea... is this a normative statement about the user API,
> or what?

Well, the only reason I'm mentioning the constant here is that
__SVE_VQ_MIN is defined too and has a perfectly good value, things look
a bit neater with a shared comment block. I'm not sure there's a hugely
meaingful difference between having a comment adjacent to a named
constant in a header and one a couple of lines away that mentions the
constant by name.

> My concern is that if we muddy the waters here different bits of
> software will do different things and we will get a mess with no
> advantages.

> Portability issues may ensue if userspace software feels it can
> substitute some other value for this constant, since we can't control
> what userspace uses it for.

I don't think we want people using this at all, ideally we'd remove it
but it's in the uapi.

> Would it be sufficient to say something like:

> /*
> * Yes, this is 512 QUADWORDS.
> * Never allocate memory or size structures based on the value of this
> * constant.
> */
> > #define __SVE_VQ_MAX 512

I think the fact that this vector length is more than an order of
magnitude more than is architecturally supported at present needs to be
conveyed, it's perfectly reasonable for people to not want to do dynamic
allocation/sizing of buffers in some applications and the above sounds
more like stylistic guidance about using dynamic sizing to improve
memory usage.

> Though comments might be better placed alongsize SVE_VQ_MAX at al., in
> ptrace.h and sigcontext.h rather than here. The leading __ should at
> least be a hint that __SVE_VQ_MAX shouldn't be used directly by
> anyone...

Yeah, the multiple layers of indirection aren't helpful here. We
probably need to comment it in both places TBH.

Attachment: signature.asc
Description: PGP signature