Re: [riot-devel] sched_active_thread is a volatile pointer, but volatile is ignored
Hi everyone,
> 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 think the reason might be that the compiler reordered access to the variables.
See C99 standard section 5.1.3 §5:
> The least requirements on a conforming implementation are:
> — At sequence points, volatile objects are stable in the sense that previous
>accesses are complete and subsequent accesses have not yet occurred.
> [...]
See also section 6.8 §4:
> [...] The end of a full expression is a sequence point.
So e.g. a call to `thread_yield_higher()` would be a sequence point, so all
memory accesses to variable declared `volatile` written before that call would
be actually executed before, and all accesses after would have to be executed
afterwards.
So regarding to the `volatile` variable the call to `thread_yield_higher()`
would be a memory barrier (but not regarding to every other variable). And this
is exactly why `volatile` solves a problem here.
I think getting rid of `volatile` and adding "proper" memory barriers to places
where context switches are supposed to happen will make the code more robust and
might allow the compiler to optimize the code better: More robust, because
some memory accesses near context switches might have just by luck not yet been
moved by the compiler to the wrong side of a context switch - there might be
non-volatile variables that are vulnerable to being moved. And better optimized
because `volatile` will blindly turn off all optimization while accessing the
affected variables, but memory barriers are more fine-grained.
`void __sync_synchronize(void)` placed just before and just after a context
change seems to be the reasonable thing to me. (Also `core/atomic_c11.c`
conveniently provides a reasonable fallback for compilers not having it - at
least as long as the CPU does not reorder commands. But I bet this is true for
most/all IoT class CPUs as of now.)
@Kees: Still interested in this conversion? Mind to take the lead? I'm happy to
help in any way I can.
Kind regards,
Marian
On Tue, 8 Jan 2019 16:10:27 +0100
Kaspar Schleiser wrote:
> 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
> ___
> devel mailing list
> devel@riot-os.org
> https://lists.riot-os.org/mailman/listinfo/devel
pgp9Ip5Xc1mdx.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel
Re: [riot-devel] USB PID number
Hi all, Le mar. 26 févr. 2019 à 09:51, Koen Zandberg a écrit : > Hi Juan, > > On 2/25/19 3:19 PM, Juan Ignacio Carrano wrote: > > Hi all, > > > > First of all, great work. Now to the VID, PID matter: I don't think we > > should get any VID. A single PID may be ok. > > > > Product numbers are for products. RIOT is not a product. Rather, it is > > used to build product (or at least that's wath we hope for). Even if > > we obtained an ID it would be irrelevant for everyone except > > developers: if you develop a device, you should get your OWN ids, you > > cannot reuse your OS vendor's. > > > > > > > > I think that having a single PID for "Generic RIOT-powered device" (or > > something of the sort) is valuable, especially for development, and > > for the CI, and we only really need one, not a whole block. That, and > > the fact that we have a more or less large project should be enough > > justification to get a PID from pid.codes. Of course, the docs should > > clearly state that the PID is for use in RIOT development and should > > be changed for actual devices. > > > > A whole VID would not be useful: what would you do with so many PIDs? > > I agree with you here. First of all, I also don't see any use for a VID > for RIOT-os, but hey maybe somebody else has a use case for a full VID. > > For me, a hypothetical RIOT-os PID would be used only for development > and testing. CI jobs, people wanting to test USB or develop USB devices. > As soon as the USB device leaves the building it must have a different > VID/PID owned by the developer/company. Having a PID for this is mostly > for ease of development, so we don't have to use a random VID/PID with > all the consequences. A lot of USB functionality doesn't require a > specific VID/PID, but is purely recognized based on the descriptor > information. > > A second PID could be required if we have our own DFU enabled RIOT > bootloader. For this I wouldn't mind if it was used for actual products > as long as the RIOT bootloader is unmodified. This as in if it claims to > be the RIOT-os DFU bootloader, it should behave like the RIOT-os > bootloader and be able to flash RIOT-os on the mcu with the RIOT-os DFU > tooling. > > I think Koen perfectly sums up the situation and I agree with him. VID is pointless for RIOT, but having two PIDs (one for development, one for DFU) would be great. Of course, it should be clearly state that the devel PID must not be used outside of its original scope. BTW, If people want to be involve. We must port the lowlevel driver and test the stack against several MCUs (EFM32, STM32, Kinetis,etc...). Any help is welcome ! Cheers, > Cheers, > Koen > > > ___ > devel mailing list > devel@riot-os.org > https://lists.riot-os.org/mailman/listinfo/devel > -- Dylan Laduranty ___ devel mailing list devel@riot-os.org https://lists.riot-os.org/mailman/listinfo/devel
Re: [riot-devel] USB PID number
Hi Juan, On 2/25/19 3:19 PM, Juan Ignacio Carrano wrote: > Hi all, > > First of all, great work. Now to the VID, PID matter: I don't think we > should get any VID. A single PID may be ok. > > Product numbers are for products. RIOT is not a product. Rather, it is > used to build product (or at least that's wath we hope for). Even if > we obtained an ID it would be irrelevant for everyone except > developers: if you develop a device, you should get your OWN ids, you > cannot reuse your OS vendor's. > > > > I think that having a single PID for "Generic RIOT-powered device" (or > something of the sort) is valuable, especially for development, and > for the CI, and we only really need one, not a whole block. That, and > the fact that we have a more or less large project should be enough > justification to get a PID from pid.codes. Of course, the docs should > clearly state that the PID is for use in RIOT development and should > be changed for actual devices. > > A whole VID would not be useful: what would you do with so many PIDs? I agree with you here. First of all, I also don't see any use for a VID for RIOT-os, but hey maybe somebody else has a use case for a full VID. For me, a hypothetical RIOT-os PID would be used only for development and testing. CI jobs, people wanting to test USB or develop USB devices. As soon as the USB device leaves the building it must have a different VID/PID owned by the developer/company. Having a PID for this is mostly for ease of development, so we don't have to use a random VID/PID with all the consequences. A lot of USB functionality doesn't require a specific VID/PID, but is purely recognized based on the descriptor information. A second PID could be required if we have our own DFU enabled RIOT bootloader. For this I wouldn't mind if it was used for actual products as long as the RIOT bootloader is unmodified. This as in if it claims to be the RIOT-os DFU bootloader, it should behave like the RIOT-os bootloader and be able to flash RIOT-os on the mcu with the RIOT-os DFU tooling. Cheers, Koen signature.asc Description: OpenPGP digital signature ___ devel mailing list devel@riot-os.org https://lists.riot-os.org/mailman/listinfo/devel
