[riot-notifications] [RIOT-OS/RIOT] sys/pm: Correctly access pm_blocker (#13977)

Marian Buschsieweke notifications at github.com
Tue May 19 08:07:54 CEST 2020

@maribu commented on this pull request.

> +    unsigned state = irq_disable();
+    pm_blocker_t result = pm_blocker;
+    irq_restore(state);
+    return result;

There sadly is no guarantee, that the compiler will generate a single read whenever possible. There were examples on GCC generating two 32 bit stores on x86_64 when writing to a `volatile`, as two stores with an immediate where faster than preparing the correct value in a register and writing that back to memory. As a result, some linux drivers stopped working when compiled with that GCC version. I think it is safe to say that the Linux kernel guys convinced the compiler guys that their compilers have no host to run on if they break kernels by splitting a single `volatile` accesses into two accesses. But for non-`volatile` accesses we should still be careful to do any assumptions on how the generated assembly looks like.

So what we could IMO safely do is something like this:

pm_blocker_t pm_get_blocker(void)
    pm_blocker_t result;
    if (ARCH_32BIT) {
        volatile uint32_t *tmp = &pm_blocker.val_u32;
        result.val_u32 = *tmp;
    else {
        unsigned state = irq_disable();
        result = pm_blocker;
    return result;

It would be nice to make the `FEATURES` like `arch32_bit` available to C code for this. The compiler should than drop the dead branch.

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20200518/73df8bda/attachment.htm>

More information about the notifications mailing list