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.

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

Fix a race-condition in esp_ipc_isr (IDFGH-10179) #11447

Closed

Conversation

pguyot
Copy link
Contributor

@pguyot pguyot commented May 19, 2023

The race condition is very unlikely on real hardware but can be observed with qemu under heavy load.
Also add missing memw instructions which are generated by the C compiler but absent in the assembly code.

Fix #11433
See issue for discussion

@CLAassistant
Copy link

CLAassistant commented May 19, 2023

CLA assistant check
All committers have signed the CLA.

@espressif-bot espressif-bot added the Status: Opened Issue is new label May 19, 2023
@github-actions github-actions bot changed the title Fix a race-condition in esp_ipc_isr Fix a race-condition in esp_ipc_isr (IDFGH-10179) May 19, 2023
bettio added a commit to atomvm/AtomVM that referenced this pull request May 19, 2023
…tests

Fix flappiness of esp32 qemu tests

ESP32 qemu tests were flappy because of a race condition in esp-idf that
probably only occurs on qemu under heavy load (or within GitHub CI which
doesn't really allocate two cores to actions)

The fix consists in applying a patch against v4.4.4
The patch was also provided in espressif/esp-idf#11447
See discussion in espressif/esp-idf#11433

Also bump versions of pytest-embedded plugins
Also optimize esp32 test by using espressif precompiled qemu binary

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
@chipweinberger
Copy link
Contributor

When I see PR's that are so low level, I always wonder how you found this, and what you are working on? Impressive work.

The race condition is very unlikely on real hardware but can be observed with
qemu under heavy load.
Also add missing `memw` instructions which are generated by the C compiler but
absent in the assembly code.

Signed-off-by: Paul Guyot <[email protected]>
@pguyot pguyot force-pushed the w20/fix-race-condition-in-esp_ipc_isr branch from 9ed1555 to 39b74ac Compare May 20, 2023 12:02
@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels May 22, 2023
@KonstantinKondrashov
Copy link
Collaborator

Thanks for the PR. I accepted these changes. Also, I will do backports up to v4.4.

@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: In Progress Work is in progress labels May 24, 2023
espressif-bot pushed a commit that referenced this pull request May 26, 2023
The race condition is very unlikely on real hardware but can be observed with
qemu under heavy load.
Also add missing `memw` instructions which are generated by the C compiler but
absent in the assembly code.

Signed-off-by: Paul Guyot <[email protected]>
Signed-off-by: KonstantinKondrashov <[email protected]>

Merges #11447
Closes #11433
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution again, changes have been merged with ceb121f.

@pguyot pguyot deleted the w20/fix-race-condition-in-esp_ipc_isr branch June 13, 2023 05:27
espressif-bot pushed a commit that referenced this pull request Jun 15, 2023
The race condition is very unlikely on real hardware but can be observed with
qemu under heavy load.
Also add missing `memw` instructions which are generated by the C compiler but
absent in the assembly code.

Signed-off-by: Paul Guyot <[email protected]>
Signed-off-by: KonstantinKondrashov <[email protected]>

Merges #11447
Closes #11433
@AxelLin
Copy link
Contributor

AxelLin commented Jul 4, 2023

Thanks for the PR. I accepted these changes. Also, I will do backports up to v4.4.

v4.4 branch does not include this fix so far.

espressif-bot pushed a commit that referenced this pull request Jul 7, 2023
The race condition is very unlikely on real hardware but can be observed with
qemu under heavy load.
Also add missing `memw` instructions which are generated by the C compiler but
absent in the assembly code.

Signed-off-by: Paul Guyot <[email protected]>
Signed-off-by: KonstantinKondrashov <[email protected]>

Merges #11447
Closes #11433
espressif-bot pushed a commit that referenced this pull request Jul 14, 2023
The race condition is very unlikely on real hardware but can be observed with
qemu under heavy load.
Also add missing `memw` instructions which are generated by the C compiler but
absent in the assembly code.

Signed-off-by: Paul Guyot <[email protected]>
Signed-off-by: KonstantinKondrashov <[email protected]>

Merges #11447
Closes #11433
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Race condition in esp_ipc_isr_stall_other_cpu/esp_ipc_isr_release_other_cpu (IDFGH-10162)
7 participants