From f6521a360dc0a7d370714184afea39c93d177eb5 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Wed, 25 Jul 2018 11:28:43 -0700 Subject: [PATCH] kernel/thread_abort: Remove needless locking The two APIs protected by this lock are themselves internally synchronized. Replace the irq_lock with a spinlock anyway, because what I think it's doing is trying to prevent a race where something else like an ISR or something it wakes up mucks with the thread before this completes. Seems fragile on SMP as it stands, but this preserves behavior on uniprocessor architectures. Signed-off-by: Andy Ross --- kernel/thread_abort.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/kernel/thread_abort.c b/kernel/thread_abort.c index f739171ce43..14e1967d508 100644 --- a/kernel/thread_abort.c +++ b/kernel/thread_abort.c @@ -27,9 +27,15 @@ extern void _k_thread_single_abort(struct k_thread *thread); #if !defined(CONFIG_ARCH_HAS_THREAD_ABORT) void _impl_k_thread_abort(k_tid_t thread) { - unsigned int key; - - key = irq_lock(); + /* We aren't trying to synchronize data access here (these + * APIs are internally synchronized). The original lock seems + * to have been in place to prevent the thread from waking up + * due to a delivered interrupt. Leave a dummy spinlock in + * place to do that. This API should be revisted though, it + * doesn't look SMP-safe as it stands. + */ + struct k_spinlock lock = {}; + k_spinlock_key_t key = k_spin_lock(&lock); __ASSERT((thread->base.user_options & K_ESSENTIAL) == 0, "essential thread aborted"); @@ -37,7 +43,7 @@ void _impl_k_thread_abort(k_tid_t thread) _k_thread_single_abort(thread); _thread_monitor_exit(thread); - _reschedule_irqlock(key); + _reschedule(&lock, key); } #endif