NotificationsYou must be signed in to change notification settings
Fork8.2k
Star20.4k

Commit3586586

committed

rp2/mpthreadport: Fix atomic section.

Prior to this commit there is a potential deadlock inmp_thread_begin_atomic_section(), when obtaining the atomic_mutex, in thefollowing situation:- main thread calls mp_thread_begin_atomic_section() (for whatever reason, doesn't matter)- the second core is running so the main thread grabs the mutex via the call mp_thread_mutex_lock(&atomic_mutex, 1), and this succeeds- before the main thread has a chance to run save_and_disable_interrupts() a USB IRQ comes in and the main thread jumps off to process this IRQ- that USB processing triggers a call to the dcd_event_handler() wrapper from commitbcbdee2- that then calls mp_sched_schedule_node()- that then attempts to obtain the atomic section, calling mp_thread_begin_atomic_section()- that call then blocks trying to obtain atomic_mutex- core0 is now deadlocked on itself, because the main thread has the mutex but the IRQ handler (which preempted the main thread) is blocked waiting for the mutex, which will never be freeThe solution in this commit is to use mutex enter/exit functions that alsoatomically disable/restore interrupts.Fixes issues#12980 and#13288.Signed-off-by: Damien George <damien@micropython.org>

1 parenteadae37 commit3586586Copy full SHA for 3586586

File tree

1 file changed

+10

-9

lines changed

ports/rp2
- mpthreadport.c

1 file changed

+10

-9

lines changed

`‎ports/rp2/mpthreadport.c`

Lines changed: 10 additions & 9 deletions

Original file line number	Diff line number	Diff line change
`@@ -30,6 +30,7 @@`
`30`	`30`	`#include"py/mpthread.h"`
`31`	`31`	`#include"pico/stdlib.h"`
`32`	`32`	`#include"pico/multicore.h"`
	`33`	`+#include"mutex_extra.h"`
`33`	`34`
`34`	`35`	`#ifMICROPY_PY_THREAD`
`35`	`36`
`@@ -45,31 +46,31 @@ STATIC uint32_t *core1_stack = NULL;`
`45`	`46`	`STATICsize_tcore1_stack_num_words=0;`
`46`	`47`
`47`	`48`	`// Thread mutex.`
`48`		`-STATICmp_thread_mutex_tatomic_mutex;`
	`49`	`+STATICmutex_tatomic_mutex;`
`49`	`50`
`50`	`51`	`uint32_tmp_thread_begin_atomic_section(void) {`
`51`	`52`	`if (core1_entry) {`
`52`	`53`	`// When both cores are executing, we also need to provide`
`53`	`54`	`// full mutual exclusion.`
`54`		`-mp_thread_mutex_lock(&atomic_mutex,1);`
	`55`	`+returnmutex_enter_blocking_and_disable_interrupts(&atomic_mutex);`
	`56`	`+ }else {`
	`57`	`+returnsave_and_disable_interrupts();`
`55`	`58`	`}`
`56`		`-`
`57`		`-returnsave_and_disable_interrupts();`
`58`	`59`	`}`
`59`	`60`
`60`	`61`	`voidmp_thread_end_atomic_section(uint32_tstate) {`
`61`		`-restore_interrupts(state);`
`62`		`-`
`63`		`-if (core1_entry) {`
`64`		`-mp_thread_mutex_unlock(&atomic_mutex);`
	`62`	`+if (atomic_mutex.owner!=LOCK_INVALID_OWNER_ID) {`
	`63`	`+mutex_exit_and_restore_interrupts(&atomic_mutex,state);`
	`64`	`+}else {`
	`65`	`+restore_interrupts(state);`
`65`	`66`	`}`
`66`	`67`	`}`
`67`	`68`
`68`	`69`	`// Initialise threading support.`
`69`	`70`	`voidmp_thread_init(void) {`
`70`	`71`	`assert(get_core_num()==0);`
`71`	`72`
`72`		`-mp_thread_mutex_init(&atomic_mutex);`
	`73`	`+mutex_init(&atomic_mutex);`
`73`	`74`
`74`	`75`	`// Allow MICROPY_BEGIN_ATOMIC_SECTION to be invoked from core1.`
`75`	`76`	`multicore_lockout_victim_init();`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Commit3586586

File tree

1 file changed

1 file changed

`‎ports/rp2/mpthreadport.c`

0 commit comments