lwIP - sys_thread_new() stack size and lack of assert?

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

lwIP - sys_thread_new() stack size and lack of assert?

5,388 Views
dmarks_ls
Senior Contributor I

So I'm integrating Ethernet support into my RT1050 application (SDK 2.6.0), and I'm making sure that all my tasks have enough stack allocated.  I'm looking at the implementation of sys_thread_new() in lwip/port/sys_arch.c:

/*---------------------------------------------------------------------------*
 * Routine:  sys_thread_new
 *---------------------------------------------------------------------------*
 * Description:
 *      Starts a new thread with priority "prio" that will begin its
 *      execution in the function "thread()". The "arg" argument will be
 *      passed as an argument to the thread() function. The id of the new
 *      thread is returned. Both the id and the priority are system
 *      dependent.
 * Inputs:
 *      char *name              -- Name of thread
 *      void (* thread)(void *arg) -- Pointer to function to run.
 *      void *arg               -- Argument passed into function
 *      int stacksize           -- Required stack amount in bytes
 *      int prio                -- Thread priority
 * Outputs:
 *      sys_thread_t            -- Pointer to per-thread timeouts.
 *---------------------------------------------------------------------------*/
sys_thread_t sys_thread_new( const char *pcName, 
  void( *pxThread )( void *pvParameters ), void *pvArg, int iStackSize, int iPriority )
{
    TaskHandle_t xCreatedTask;
    portBASE_TYPE xResult;
    sys_thread_t xReturn;
    xResult = xTaskCreate( pxThread, pcName, iStackSize, pvArg, iPriority, &xCreatedTask );
    [...]
}
‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

What's confusing is that the parameter iStackSize ("stacksize") is documented above as "Required stack amount in bytes", but xTaskCreate() infamously allocates stack by numbers of words, not bytes, and iStackSize is being passed directly to xTaskCreate().  So if I pass in 3000 to sys_thread_new(), I'm going to get a task with a 12,000 byte stack size.

So either iStackSize needs to be divided by sizeof(StackType_t) before being passed to xTaskCreate(), or the documentation for sys_thread_new() needs to be revised.  Given the number of implementations that would likely break if stack sizes were suddenly reduced by 75%, I would vote for the latter.

Also, just noticed in some of the documentation for lwip -- they state that sys_thread_new() must succeed, even though it returns a result, and that sys_thread_new() should assert that the task was created.  Based on that documentation, I suggest that sys_thread_new() should appear as follows:

sys_thread_t sys_thread_new( const char *pcName,
  void( *pxThread )( void *pvParameters ), void *pvArg, int iStackSize, int iPriority )
{
    TaskHandle_t xCreatedTask;
    portBASE_TYPE xResult;
    sys_thread_t xReturn;

    xResult = xTaskCreate( pxThread, pcName, iStackSize, pvArg, iPriority, &xCreatedTask );
    assert( xResult == pdPASS );
    xReturn = xCreatedTask;
    return xReturn;
}
Tags (3)
0 Kudos
1 Reply

4,745 Views
danielchen
NXP TechSupport
NXP TechSupport

Hi David:

I reported this issue to the soft team.

Thank you very much for your feedback.

Regards

Daniel