Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pico_atomic: atomic operations are not multicore-atomic #1805

Closed
geurtv opened this issue Aug 12, 2024 · 2 comments
Closed

pico_atomic: atomic operations are not multicore-atomic #1805

geurtv opened this issue Aug 12, 2024 · 2 comments
Assignees
@kilograham
Milestone
2.1.0

Comments

@geurtv
Copy link
Contributor

geurtv commented Aug 12, 2024

From SDK 2.0.0 release notes:

Additional support for C11 atomic functions using spin lock number PICO_SPINLOCK_ID_ATOMIC.

and

On RP2040, all functions are implemented via spinlock.

Well ... no? PICO_SPINLOCK_ID_ATOMIC doesn't seem to be used anywhere in the pico-sdk sources and spinlocks are nowhere to be found in atomic.c.

The atomic operations in atomic.c call atomic_lock() which itself only calls save_and_disable_interrupts(), and if I'm not mistaken this only disables interrupts on the current cpu core.
@kilograham
Copy link
Contributor

Wow... what? how embarrassing... there was some merging of different implementations... clearly something went wrong in the merge!!!

@kilograham kilograham added this to the 2.0.1 milestone Aug 12, 2024
@kilograham kilograham self-assigned this Aug 12, 2024
kilograham added a commit that referenced this issue Aug 16, 2024
@kilograham
#1805 fix merge issue in pico_atomic which ended up with using IRQ di…
34b0d53 
…sble instead of spin lock
kilograham added a commit that referenced this issue Aug 19, 2024
@kilograham
#1805 fix merge issue in pico_atomic which ended up with using IRQ di…
62df126 
…sble instead of spin lock (#1838)
@kilograham
Copy link
Contributor

kilograham commented Aug 19, 2024 •
edited by lurch
Loading

#1838 merged into develop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kilograham kilograham

Labels
None yet
Projects
None yet
Milestone
2.1.0
Development

No branches or pull requests
2 participants
@kilograham @geurtv