sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Merged in masayuki2009/nuttx.nuttx/fix_smp_bugs (pull request #615)

Browse Source 
Fix SMP related bugs

* sched/sched: Fix a deadlock in SMP mode

    Two months ago, I introduced sched_tasklist_lock() and
    sched_tasklist_unlock() to protect tasklists in SMP mode.
    Actually, this change works pretty well for HTTP audio
    streaming aging test with lc823450-xgevk.

    However, I found a deadlock in the scheduler when I tried
    similar aging tests with DVFS autonomous mode where CPU
    clock speed changed based on cpu load. In this case, call
    sequences were as follows;

    cpu1: sched_unlock()->sched_mergepending()->sched_addreadytorun()->up_cpu_pause()
    cpu0: sched_lock()->sched_mergepending()

    To avoid this deadlock, I added sched_tasklist_unlock() when calling
    up_cpu_pause() and sched_addreadytorun(). Also, added
    sched_tasklist_lock() after the call.

    Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>

* libc: Add critical section in lib_filesem.c for SMP

    To set my_pid into fs_folder atomically in SMP mode,
    critical section API must be used.

    Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>

* mm: Add critical section in mm_sem.c for SMP

    To set my_pid into mm_folder atomically in SMP mode,
    critical section API must be used.

    Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>

* net: Add critical section in net_lock.c for SMP

    To set my pid (me) into fs_folder atomically in SMP mode,
    critical section API must be used.

    Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>

Approved-by: Gregory Nutt <gnutt@nuttx.org>
This commit is contained in:
Masayuki Ishikawa 2018-03-20 12:34:38 +00:00 committed by Gregory Nutt
parent 278cc6f70a
commit 1d958980bd
6 changed files with 72 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
libc/misc/lib_filesem.c
View File

@ -48,6 +48,10 @@
#include <nuttx/semaphore.h>
#include <nuttx/fs/fs.h>
#ifdef CONFIG_SMP
# include <nuttx/irq.h>
#endif
#include "libc.h"
#ifndef CONFIG_STDIO_DISABLE_BUFFERING
@ -78,6 +82,10 @@ void lib_sem_initialize(FAR struct file_struct *stream)
void lib_take_semaphore(FAR struct file_struct *stream)
{
#ifdef CONFIG_SMP
irqstate_t flags = enter_critical_section();
#endif
pid_t my_pid = getpid();
int ret;
@ -108,6 +116,10 @@ void lib_take_semaphore(FAR struct file_struct *stream)
stream->fs_holder = my_pid;
stream->fs_counts = 1;
}
#ifdef CONFIG_SMP
leave_critical_section(flags);
#endif
}
/****************************************************************************
@ -116,6 +128,10 @@ void lib_take_semaphore(FAR struct file_struct *stream)
void lib_give_semaphore(FAR struct file_struct *stream)
{
#ifdef CONFIG_SMP
irqstate_t flags = enter_critical_section();
#endif
/* I better be holding at least one reference to the semaphore */
DEBUGASSERT(stream->fs_holder == getpid());
@ -136,6 +152,10 @@ void lib_give_semaphore(FAR struct file_struct *stream)
stream->fs_counts = 0;
DEBUGVERIFY(_SEM_POST(&stream->fs_sem));
}
#ifdef CONFIG_SMP
leave_critical_section(flags);
#endif
}
#endif /* CONFIG_STDIO_DISABLE_BUFFERING */

32
mm/mm_heap/mm_sem.c
View File

@ -47,6 +47,10 @@
#include <nuttx/semaphore.h>
#include <nuttx/mm/mm.h>
#ifdef CONFIG_SMP
# include <nuttx/irq.h>
#endif
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
@ -128,6 +132,9 @@ void mm_seminitialize(FAR struct mm_heap_s *heap)
int mm_trysemaphore(FAR struct mm_heap_s *heap)
{
#ifdef CONFIG_SMP
irqstate_t flags = enter_critical_section();
#endif
pid_t my_pid = getpid();
int ret;
@ -138,7 +145,7 @@ int mm_trysemaphore(FAR struct mm_heap_s *heap)
/* Yes, just increment the number of references that I have */
heap->mm_counts_held++;
return OK;
ret = OK;
}
else
{
@ -148,15 +155,21 @@ int mm_trysemaphore(FAR struct mm_heap_s *heap)
if (ret < 0)
{
_SEM_GETERROR(ret);
return ret;
goto errout;
}
/* We have it. Claim the heap and return */
heap->mm_holder = my_pid;
heap->mm_counts_held = 1;
return OK;
ret = OK;
}
errout:
#ifdef CONFIG_SMP
leave_critical_section(flags);
#endif
return ret;
}
/****************************************************************************
@ -170,6 +183,9 @@ int mm_trysemaphore(FAR struct mm_heap_s *heap)
void mm_takesemaphore(FAR struct mm_heap_s *heap)
{
#ifdef CONFIG_SMP
irqstate_t flags = enter_critical_section();
#endif
pid_t my_pid = getpid();
/* Do I already have the semaphore? */
@ -216,6 +232,9 @@ void mm_takesemaphore(FAR struct mm_heap_s *heap)
heap->mm_counts_held = 1;
}
#ifdef CONFIG_SMP
leave_critical_section(flags);
#endif
mseminfo("Holder=%d count=%d\n", heap->mm_holder, heap->mm_counts_held);
}
@ -229,6 +248,9 @@ void mm_takesemaphore(FAR struct mm_heap_s *heap)
void mm_givesemaphore(FAR struct mm_heap_s *heap)
{
#ifdef CONFIG_SMP
irqstate_t flags = enter_critical_section();
#endif
#if defined(CONFIG_DEBUG_ASSERTIONS) || \
(defined(MONITOR_MM_SEMAPHORE) && defined(CONFIG_DEBUG_INFO))
pid_t my_pid = getpid();
@ -258,4 +280,8 @@ void mm_givesemaphore(FAR struct mm_heap_s *heap)
heap->mm_counts_held = 0;
DEBUGVERIFY(_SEM_POST(&heap->mm_semaphore));
}
#ifdef CONFIG_SMP
leave_critical_section(flags);
#endif
}

18
net/utils/net_lock.c
View File

@ -131,6 +131,9 @@ void net_lockinitialize(void)
void net_lock(void)
{
#ifdef CONFIG_SMP
irqstate_t flags = enter_critical_section();
#endif
pid_t me = getpid();
/* Does this thread already hold the semaphore? */
@ -152,6 +155,10 @@ void net_lock(void)
g_holder = me;
g_count = 1;
}
#ifdef CONFIG_SMP
leave_critical_section(flags);
#endif
}
/****************************************************************************
@ -170,6 +177,9 @@ void net_lock(void)
void net_unlock(void)
{
#ifdef CONFIG_SMP
irqstate_t flags = enter_critical_section();
#endif
DEBUGASSERT(g_holder == getpid() && g_count > 0);
/* If the count would go to zero, then release the semaphore */
@ -188,6 +198,10 @@ void net_unlock(void)
g_count--;
}
#ifdef CONFIG_SMP
leave_critical_section(flags);
#endif
}
/****************************************************************************
@ -209,12 +223,14 @@ void net_unlock(void)
int net_timedwait(sem_t *sem, FAR const struct timespec *abstime)
{
pid_t me = getpid();
unsigned int count;
irqstate_t flags;
int ret;
flags = enter_critical_section(); /* No interrupts */
pid_t me = getpid();
sched_lock(); /* No context switches */
if (g_holder == me)
{

2
sched/sched/sched_addreadytorun.c
View File

@ -275,7 +275,9 @@ bool sched_addreadytorun(FAR struct tcb_s *btcb)
if (cpu != me)
{
sched_tasklist_unlock(lock);
DEBUGVERIFY(up_cpu_pause(cpu));
lock = sched_tasklist_lock();
}
/* Add the task to the list corresponding to the selected state

2
sched/sched/sched_mergepending.c
View File

@ -242,7 +242,9 @@ bool sched_mergepending(void)
/* Add the pending task to the correct ready-to-run list. */
sched_tasklist_unlock(lock);
ret |= sched_addreadytorun(tcb);
lock = sched_tasklist_lock();
/* This operation could cause the scheduler to become locked.
* Check if that happened.

2
sched/sched/sched_removereadytorun.c
View File

@ -187,7 +187,9 @@ bool sched_removereadytorun(FAR struct tcb_s *rtcb)
me = this_cpu();
if (cpu != me)
{
sched_tasklist_unlock(lock);
DEBUGVERIFY(up_cpu_pause(cpu));
lock = sched_tasklist_lock();
}
/* The task is running but the CPU that it was running on has been