Re: racy acccess in kern_runq.c

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 06/12/2019 à 11:28, Andrew Doran a écrit :
> On Fri, Dec 06, 2019 at 10:27:20AM +0100, Maxime Villard wrote:
> 
>> With 'worker_ci', there is an actual safety issue, because the compiler could
>> split the accesses and the hardware may not use atomics by default like x86.
>> This could cause random page faults; so it needs to be strictly atomic.
> 
> No I don't accept that.
> 
> The ability to load and store a native word sized int (and in more recent
> years a pointer) with a single instruction is a fundamental assumption that
> every operating system written in C rests upon.

1) That it is done with a single instruction, does not necessarily mean it is
atomic. There are many stupid things microcodes are allowed to do (not on x86,
however).

2) That there is a one-line assignment of a variable in C, does not necessarily
mean that there will be one move instruction. There are many things the compiler
is allowed to do in optimization passes, as said already.

3) "every operating system written in C rests upon" I have some difficulty with
that. Linux has our equivalent of relaxed atomics (called {READ,WRITE}_ONCE),
with 1000+ combined references in their tree. So no, here's a big open source
kernel that does not rest upon unclear assumptions for lockless operations. No
way to check Windows, but I wouldn't be surprised if they had a similar
approach.

> If the compiler splits one of those acceses, then you are either using some
> other data type, or have a broken compiler on your hands.

Or you're just using a compiler that knows well enough how to arrange
instructions for optimized pipelining, and this compiler decides to re-order
the accesses based on desirable performance properties, as allowed by the
standard.

> https://www.gnu.org/software/libc/manual/html_node/Atomic-Types.html

I don't see anything in this link that is really reassuring. "you can assume"
that things are mostly ok on "systems we know of".

At least the rationale for {READ,WRITE}_ONCE is based on standards, and not
wild assumptions:

	https://github.com/google/ktsan/wiki/READ_ONCE-and-WRITE_ONCE

To me, Kengo's 2nd patch remains needed.

Maxime