Re: [PATCH] RISC-V: add riscv64 architecture support

[Markus Wichmann <nullplan@gmx.net>]

On Mon, Jun 03, 2019 at 04:46:56PM -0400, Drew DeVault wrote:
> Thanks for your patience, needed to wait until I returned from travel to
> take another look here.
>
> On Sun May 26, 2019 at 12:31 AM Rich Felker wrote:
> > I don't understand why a_cas is defined here, and it's almost surely
> > wrong since it has no barriers whatsoever. If a_cas is left undefined,
> > it would be auto-defined in terms of a_ll and a_sc. But if we want to
> > follow the ISA docs which place restrictions on the positioning of
> > ll/sc and what can be done between them, I think we can't define a_ll
> > and a_sc, but have to define a_cas (with proper barriers) and let
> > everything else get defined in terms of it...
>
> I don't grok the atomics code, to be honest. I'm not sure what the
> resolution is here.
>

The main atomics functions are those you find for instance in x86_64's
atomic.h. Those are used by the main library. We used to have that set
of atomics for every arch, however, we noticed that a large number of
archs are RISC archs, which basically all use the LL/SC approach. So, we
generalized that approach in src/internal/atomics.h, so now you only
need a_ll() and a_sc() (and the proper barriers a_pre_llsc() and
a_post_llsc()).

As for what barriers you need: If in doubt, use too large a barrier and
scale it back once the performance impact is too great. Better to solve
a performance problem than a concurrency problem, believe me. You
generally want a write barrier before and a read barrier after the
transaction, but if you start with a full memory barrier, you can always
scale it back later.

> > Is 2k min correct here? It's ABI so we should aim to get it right to
> > begin with.
>
> The spec doesn't recommend a minimum stack size afaict, so this as
> reasonable as any other number to me.
>

The minimum stack size must be able to contain a kernel signal context
even with SA_SIGINFO. We recently had an issue where PowerPC was not
happy with the 2k, since it can use a little bit more. So that's why its
minimum is now at 4k.

Ciao,
Markus