[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:

```C
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;
        irq_restore(state);
    }
    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:
https://github.com/RIOT-OS/RIOT/pull/13977#discussion_r427050457
-------------- 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