Possible bug in _CORE_mutex_Seize()
strauman at SLAC.Stanford.EDU
Tue Sep 30 22:19:30 UTC 2003
Joel Sherrill wrote:
> Phil Torre wrote:
>> I'm running now with some of the changes suggested by Till and Joel:
>> 1. "Paranoia" added to _CORE_mutex_Seize() which checks
>> _Thread_Dispatch_disable_level and throws an internal error if
>> its nonzero when it shouldn't be.
> Be careful with this one. I don't think this is the right place to
> check it. You can attempt to obtain a mutex in an ISR and
> _Thread_Dispatch_disable_level will definitely be non-zero. You
> can't use the Allocator Mutex when _Thread_Dispatch_disable_level
> is non-zero but it is conceivable that you could use another mutex.
??? What is a legal scenario to obtain a CORE_mutex in an ISR - especially
with _wait == TRUE -- and even if it is FALSE, I doubt it's OK. The ISR
would lock a mutex for the interrupted task.
Plus: CORE_mutex_Seize() is always executed with IRQs disabled - it's hard
to imagine that it should be legal to call it from an ISR.
The simplest scenario goes like this (summarizing):
if ( !_CORE_mutex_Is_locked( the_mutex ) )
the_mutex->holder = _Thread_Executing;
/* set other fields in the_mutex and _Thread_Executing */
It is *legal* however, to call CORE_mutex_Seize() from a dispatching
disabled section (in a task context) provided that "_wait == FALSE", a case
which is tolerated by the paranoia check. Of course, the caller is expected
to check/convert the ("indirect") return code before proceeding.
I added the check at the top of CORE_mutex_Seize() just *because*
I'd like to catch other possible misuses of mutexes.
So far, I caught no other one running a rather complex EPICS application.
> I would take this out.
>> 2. The reent struct is now allocated from the workspace using
>> _Workspace_Allocate() from libc_create_hook rather than the start
>> hook. If _Workspace_Allocate() returns NULL, the create hook
>> returns false.
>> This seems to be working (given several minutes of exhaustive testing),
>> but I notice a few things: Since I'm now calling _Workspace_Allocate()
>> directly, _RTEMS_Allocator_mutex is not being locked. That should be
>> OK, because _Thread_Dispatch_disable_level is nonzero, correct?
> It should be right because _Workspace_Allocate() is a normal part of
> rtems_task_create(). The stack, FP context, and API extensions
> are allocated from the Workspace as part of task creation.
More information about the users