Message-ID: <5fbdfe75-bd68-007b-1b64-37334a10a8d1@sholland.org>
Date:   Wed, 14 Dec 2022 17:13:34 -0600
From:   Samuel Holland <samuel@...lland.org>
To:     Vineet Gupta <vineetg@...osinc.com>,
        Palmer Dabbelt <palmer@...belt.com>, fweimer@...hat.com,
        Andrew Waterman <andrew@...ive.com>, stillson@...osinc.com,
        Paul Walmsley <paul.walmsley@...ive.com>, anup@...infault.org,
        atishp@...shpatra.org, guoren@...nel.org,
        Conor Dooley <conor.dooley@...rochip.com>,
        greentime.hu@...ive.com, vincent.chen@...ive.com,
        andy.chiu@...ive.com, arnd@...nel.org,
        linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
        bjorn@...nel.org, libc-alpha@...rceware.org,
        christoph.muellner@...ll.eu, Aaron Durbin <adurbin@...osinc.com>,
        linux@...osinc.com
Subject: Re: RISCV Vector unit disabled by default for new task (was Re:
 [PATCH v12 17/17] riscv: prctl to enable vector commands)

On 12/14/22 14:07, Vineet Gupta wrote:
> On 12/13/22 08:43, Darius Rad wrote:
>> On Fri, Dec 09, 2022 at 11:42:19AM -0800, Vineet Gupta wrote:
>>> If that is failing,
>>> the system is not doing well anyways. Besides it is not an issue at all
>>> since ENOMEM in clone/execve for the additional space should handle the
>>> failure anyways. Only very sophisticated apps would downgrade from
>>> executing
>>> V to Scalar code if the prctl failed.
>> This seems unlikely.  As vector support does not exist in any present
>> hardware, and the vector extension is only optional in the RISC-V
>> profiles
>> that include it, I would think that it is almost certain that any
>> application that supports V would have a fallback path for when the V
>> extension is not available.
> 
> For specialized cases sure we would expect fat binaries with IFUNC based
> dispatches (glibc mem*/str* are obvious examples). But with newer
> compilers autovec is increasing becoming default even at medium
> optimization levels such as -O2. So V code littered all over is just a
> matter of time for the profiles/variants which allow V. For less capable

Autovectorization is only possible where the profile *requires* V. And
no existing profile does that.

> systems w/o V this is all but moot discussion since kernel itself need
> not be built with V enabled.

Distro kernels will be built with V enabled, and will spend most of
their time running on systems without V.

>> Another motivation for requiring that user space request use of the
>> vector
>> extension is that the vector unit may be shared between multiple harts
>> and/or have power or performance implications in the system.  By
>> requiring
>> that user space request access, it allows the system to decline that
>> access, and user space can handle this gracefully.
> 
> But in this specific example won't the prctl cause more pain. So 2
> concurrent processes on 2 different harts with shared V unit. One sends
> prctl to enable and other wants to disable, what would the kernel do.
> Will it just be whoever ends up running later wins. Granted I'm not too
> familiar with how such a cross-hart sharing would work in a Vector
> instructions being part of ISA  (vs. Vector accelerator with job
> push/pull approach)

The vector execution unit can be shared between harts, not the vector
state. Think SMT.

> Honestly I'm sympathetic to your power concerns with keeping V enabled
> all the time. But the mechanics of implementing this prctl makes me
> wary. Assuming this is done from dynamic loader, it implies loader
> itself needs to be built with V disabled. This also leaves bunch of perf
> on table since loader does tons of of string and memory operations which
> could potentially benefit from V enabled code, granted it is not deal
> breaker.

The dynamic loader can do dynamic dispatch based on the result of the
prctl() just as well as any other library.

And the distro's dynamic loader can't be compiled with autovectorization
enabled anyway, because again the profiles support processors without V.

>> If we add a mechanism for user space to request access to the vector
>> extension, and it turns out that it was unnecessary, the worst that has
>> happened is a slight inconvenience.
>>
>> If we do not add such a mechanism, and later determine that it is
>> necessary, we have a much greater problem.  There would be backward
>> compatibility issues with the ABI, and such a mechanism could probably
>> not
>> be fully implemented at all due to the desire to support potential future
>> legacy vector code.
> 
> Very true, but this in itself is not sufficient of a reason to warrant
> adding it now.

It is, because changing the sigcontext layout without an opt-in is
already an ABI break (it completely blows past MINSIGSTKSZ).

Regards,
Samuel

Powered by blists - more mailing lists