Possible bug in _CORE_mutex_Seize()

[Till Straumann]

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 */
   _ISR_enable(level);
return 0;

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.

T.

> 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?
> 
> 
> 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.
> 
> 
> 
>> -Phil
>>
> 
>