[riot-devel] sched_active_thread is a volatile pointer, but volatile is ignored

Kaspar Schleiser kaspar at schleiser.de
Tue Jan 8 16:10:27 CET 2019


Hi Kees,

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.

This was certainly a case of only (at most) half-understanding volatile,
which then turned into "if it is not broken, don't fix it".

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.

Kaspar


More information about the devel mailing list