mm/mm_heap/mm_sem.c and sched/task/task_getpid.c: Commits 43d37c866b and f37202cbc0 resolved some problems with the original fix of 91aa26774b. However, Those changes used an internal OS interface (sched_self()) which is not available when the the user-space memory manager is built in the PROTECTED or KERNEL builds and resulted in build failures.

This commit repartitions the logic by moving some of the changes from mm_sem.c into task_getpid.c.  The logic is equivalent for the case of mm_trysemaphore(), but no has wider impact since it potentially affects all callers of getpid().  Hence, this change may also introduce some other issues that will need to be addressed.
This commit is contained in:
Gregory Nutt 2018-12-30 08:41:06 -06:00
parent 30ddb909c0
commit 40cdc187bb
2 changed files with 66 additions and 26 deletions

View File

@ -1,7 +1,8 @@
/**************************************************************************** /****************************************************************************
* mm/mm_heap/mm_sem.c * mm/mm_heap/mm_sem.c
* *
* Copyright (C) 2007-2009, 2013, 2017 Gregory Nutt. All rights reserved. * Copyright (C) 2007-2009, 2013, 2017-2018 Gregory Nutt. All rights
* reserved.
* Author: Gregory Nutt <gnutt@nuttx.org> * Author: Gregory Nutt <gnutt@nuttx.org>
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
@ -44,7 +45,6 @@
#include <errno.h> #include <errno.h>
#include <assert.h> #include <assert.h>
#include <nuttx/sched.h>
#include <nuttx/semaphore.h> #include <nuttx/semaphore.h>
#include <nuttx/mm/mm.h> #include <nuttx/mm/mm.h>
@ -76,6 +76,14 @@
# define _SEM_GETERROR(r) (r) = -errno # define _SEM_GETERROR(r) (r) = -errno
#endif #endif
/* This is a special value that indicates that there is no holder of the
* semaphore. The valid range of PIDs is 0-32767 and any value outside of
* that range could be used (except -ESRCH which is a special return value
* from getpid())
*/
#define NO_HOLDER ((pid_t)-1)
/* Define MONITOR_MM_SEMAPHORE to enable semaphore state monitoring */ /* Define MONITOR_MM_SEMAPHORE to enable semaphore state monitoring */
#ifdef MONITOR_MM_SEMAPHORE #ifdef MONITOR_MM_SEMAPHORE
@ -115,7 +123,7 @@ void mm_seminitialize(FAR struct mm_heap_s *heap)
(void)nxsem_init(&heap->mm_semaphore, 0, 1); (void)nxsem_init(&heap->mm_semaphore, 0, 1);
heap->mm_holder = -1; heap->mm_holder = NO_HOLDER;
heap->mm_counts_held = 0; heap->mm_counts_held = 0;
} }
@ -135,17 +143,16 @@ int mm_trysemaphore(FAR struct mm_heap_s *heap)
#ifdef CONFIG_SMP #ifdef CONFIG_SMP
irqstate_t flags = enter_critical_section(); irqstate_t flags = enter_critical_section();
#endif #endif
FAR struct tcb_s *rtcb = sched_self(); pid_t my_pid = getpid();
pid_t my_pid;
int ret; int ret;
/* sched_self() returns the TCB at the head of the ready-to-run list. /* getpid() returns the task ID of the task at the head of the ready-to-
* mm_trysemaphore() may be called during context switches. There are * run task list. mm_trysemaphore() may be called during context
* certain situations during context switching when the the OS data * switches. There are certain situations during context switching when
* structures are in flux and where the current task (i.e., the task at * the OS data structures are in flux and where the current task (i.e.,
* the head of the ready-to-run task list) is not actually running. * the task at the head of the ready-to-run task list) is not actually
* Granting the semaphore access in that case is known to result in heap * running. Granting the semaphore access in that case is known to result
* corruption as in the following failure scenario. * in heap corruption as in the following failure scenario.
* *
* ---------------------------- ------------------------------- * ---------------------------- -------------------------------
* TASK A TASK B * TASK A TASK B
@ -161,14 +168,18 @@ int mm_trysemaphore(FAR struct mm_heap_s *heap)
* entered, and * entered, and
* - Heap is corrupted. * - Heap is corrupted.
* ---------------------------- ------------------------------- * ---------------------------- -------------------------------
*
* This is handled by getpid(): If the case where Task B is not actually
* running, then getpid() will return the special value -ESRCH. That will
* avoid taking the fatal 'if' logic and will fall through to use the
* 'else', albeit with a nonsensical PID value.
*/ */
/* Does the current task already hold the semaphore? Is the current /* Does the current task already hold the semaphore? Is the current
* task actually running? * task actually running?
*/ */
my_pid = rtcb->pid; if (heap->mm_holder == my_pid)
if (heap->mm_holder == my_pid && rtcb->task_state == TSTATE_TASK_RUNNING)
{ {
/* Yes, just increment the number of references held by the current /* Yes, just increment the number of references held by the current
* task. * task.
@ -310,7 +321,7 @@ void mm_givesemaphore(FAR struct mm_heap_s *heap)
mseminfo("PID=%d giving\n", my_pid); mseminfo("PID=%d giving\n", my_pid);
heap->mm_holder = -1; heap->mm_holder = NO_HOLDER;
heap->mm_counts_held = 0; heap->mm_counts_held = 0;
DEBUGVERIFY(_SEM_POST(&heap->mm_semaphore)); DEBUGVERIFY(_SEM_POST(&heap->mm_semaphore));
} }

View File

@ -1,7 +1,7 @@
/**************************************************************************** /****************************************************************************
* sched/task/task_getpid.c * sched/task/task_getpid.c
* *
* Copyright (C) 2007, 2009, 2014 Gregory Nutt. All rights reserved. * Copyright (C) 2007, 2009, 2014, 2018 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org> * Author: Gregory Nutt <gnutt@nuttx.org>
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
@ -54,6 +54,21 @@
* Description: * Description:
* Get the task ID of the currently executing task. * Get the task ID of the currently executing task.
* *
* Input parameters:
* None
*
* Returned Value:
* Normally when called from user applications, getpid() will return the
* task ID of the currently executing task, that is, the task at the head
* of the ready-to-run list. There is no specification for any errors
* returned from getpid().
*
* getpid(), however, may be called from within the OS in some cases.
* There are certain situations during context switching when the OS data
* structures are in flux and where the current task at the head of the
* ready-to-run task list) is not actually running. In that case,
* getpid() will return the error: -ESRCH
*
****************************************************************************/ ****************************************************************************/
pid_t getpid(void) pid_t getpid(void)
@ -61,25 +76,39 @@ pid_t getpid(void)
FAR struct tcb_s *rtcb; FAR struct tcb_s *rtcb;
/* Get the TCB at the head of the ready-to-run task list. That /* Get the TCB at the head of the ready-to-run task list. That
* will be the currently executing task. There is an exception to * will usually be the currently executing task. There is are two
* this: Early in the start-up sequence, the ready-to-run list may be * exceptions to this:
* empty! This case, of course, the start-up/IDLE thread with pid == 0 *
* must be running. * 1. Early in the start-up sequence, the ready-to-run list may be
* empty! This case, of course, the CPU0 start-up/IDLE thread with
* pid == 0 must be running, and
* 2. As described above, during certain context-switching conditions the
* task at the head of the ready-to-run list may not actually be
* running.
*/ */
rtcb = this_task(); rtcb = this_task();
if (rtcb) if (rtcb != NULL)
{ {
/* Return the task ID from the TCB at the head of the ready-to-run /* Check if the task is actually running */
* task list
*/
return rtcb->pid; if (rtcb->task_state == TSTATE_TASK_RUNNING)
{
/* Yes.. Return the task ID from the TCB at the head of the
* ready-to-run task list
*/
return rtcb->pid;
}
/* No.. return -ESRCH to indicate this condition */
return (pid_t)-ESRCH;
} }
/* We must have been called earlier in the start up sequence from the /* We must have been called earlier in the start up sequence from the
* start-up/IDLE thread before the ready-to-run list has been initialized. * start-up/IDLE thread before the ready-to-run list has been initialized.
*/ */
return 0; return (pid_t)0;
} }