On 08-01-19 16:10, Kaspar Schleiser wrote:
Hi Kees,
Hey Kaspar,
On 1/7/19 9:19 PM, Kees Bakker wrote:
My main concern is: who made it volatile in the first place?
I did, almost a decade ago.

And what was the reasoning behind it? Volatile is one of the least
understood properties of the C language (my personal opinion). I'm
hoping that the volatile was not just thrown in because it feels good
when doing threads. And in other places the volatile is ignored,
hopefully for a good reason (optimisation is _not_ a good reason).
IIRC the intention was so the IPC code would create read / write
accesses whenever accessing fields of thread_t.

E.g.:

void msg_recv(...)
{
    if (!sched_active_thread->waiters) {
     // platform specific macro to suspend thread
    }

    thread_t *waiter = sched_active_thread->waiters;

    // ...
}

(or similar)

My understanding of volatile back then was that the compiler could,
without volatile, assume that sched_active_thread->waiters equals NULL.
Well, in the example code, the compiler would just read sched_active_thread->waiters
once and keep it in a register. If either sched_active_thread or waiters
can change in between the uses in that function you must declare some
things volatile. What is also interesting here is to ask ourselves,
can sched_active_thread change in between? If so you would need to declare
that variable as
    thread_t * volatile sched_active_thread;
(( This was mentioned in one of the Github issues, but the discussion stalled. ))

This was certainly a case of only (at most) half-understanding volatile,
Which proves my point :-)
which then turned into "if it is not broken, don't fix it".
I'm not a fan of that. It is like looking away and hoping there
are no problems. No matter how hard it is, we need to fully
understand, especially a thing like a scheduler.

Nowadays such code is always guarded with disabled IRQs.

I seem to remember that we tried making sched_active_thread non-volatile
at some point, breaking things, but that has also been a long time ago.

I'm all for removing the qualifier. But we do have to make sure to
thoroughly test core/ on all platforms.


Of course we need thorough testing. Let's not rush. If I wanted to
experiment with volatile changes, what would be good test programs
for this?
--
Kees
_______________________________________________
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel

Reply via email to