xSemaphoreTake --> crash, xSemaphoreTakeFromISR-->no crash

ypnklabs wrote on Saturday, March 18, 2017:

I have a ADC interrupt that occurs every 17ms and which copies the samples into a global static array, after which GivesFromISR to a binary semaphore so that the samples may be processed with a thread in the foreground. However, if I use xSemaphoreTake instead of xSemaphoreTakeFromISR in the thread the scheduler seems to stop. The strange part is that this only started happening after incorporating a 3rd party function call that uses the global static array modified by the ISR (closed source binary that was linked in the final binary…it does some CMSIS-DSP arm math calcs). Also, the ISR I mentioned is actually a function that gets called from the actual ISR and it is not the ISR it self.

Some code to better illustrate:

void app_drv_tasks_init(void)
{
    m_sem_samples_event_ready = xSemaphoreCreateBinary(); //static var already declared
    if (NULL == m_sem_samples_event_ready)
    {
        APP_ERROR_HANDLER(NRF_ERROR_NO_MEM);
    }

    if (pdPASS != xTaskCreate(saadc_sample_done_event_thread,
                              "SAMPLES", 256, NULL, 1,
                              &m_saadc_sample_done_thread))
    {
        APP_ERROR_HANDLER(NRF_ERROR_NO_MEM);
    }
    
    
    /** SAADC Interrupt handler */
void saadc_callback(nrf_drv_saadc_evt_t const * p_event)
{
    if (p_event->type == NRF_DRV_SAADC_EVT_DONE)
    {
        ret_code_t err_code;
        err_code = nrf_drv_saadc_buffer_convert(p_event->data.done.p_buffer, SAMPLE_COUNT);
        APP_ERROR_CHECK(err_code);

        for (int i = 0; i < SAMPLE_COUNT; i++)
        {
            m_app_buffer[i] = p_event->data.done.p_buffer[i];
        }
        saadc_sample_done_event_signal();
    }
}

uint32_t saadc_sample_done_event_signal(void)
{
    BaseType_t yield_req = pdFALSE;
    xSemaphoreGiveFromISR(m_sem_samples_event_ready, &yield_req);
    portYIELD_FROM_ISR(yield_req);
    return NRF_SUCCESS;
}

void saadc_sample_done_event_thread(void * arg)
{
    uint8_t i;
    uint8_t result;
    adc_init(); // generates interrupts, but started after scheduler
    three_party_code_init();

    for(;;)
        {
            // Need to figure out why *FromISR fixes the crash
            while (pdFALSE == xSemaphoreTakeFromISR(m_sem_samples_event_ready, portMAX_DELAY))
            {
            }
            // If this line is removed xSemaphoreTake works fine
            three_party_code(m_app_buffer, &result);
        }
}

int main(void)
{

    app_drv_tasks_init();
    
    SCB->SCR |= SCB_SCR_SLEEPDEEP_Msk;

    vTaskStartScheduler();

    while (true)
    {
        APP_ERROR_HANDLER(NRF_ERROR_FORBIDDEN);
    }
 }
    

I’m using the nRF52x with arm GCC/Makefile

davedoors wrote on Saturday, March 18, 2017:

Is configASSERT() defined? Is configCHECK_FOR_STACK_OVERFLOW set to 2? Does three_party_code() mess with the MCU interrupt registers?

Is te nRF52x a Cortex-M0?

richard_damon wrote on Saturday, March 18, 2017:

One big thing to note is that the TakeFromISR will NOT block, and that last parameter is supposed to be a pointer to a flag variable to mark if a task was woken from the Take. That means that by switching to the FromISR version, you are starving all tasks of lower priority (like Idle). Perhaps you have a bug there (like an idle hook that blocks).

ypnklabs wrote on Saturday, March 18, 2017:

I checked the Stack using the API, it appears to be ok. I also made it quite big with the same result.

Its a Arm cortex m4, nRF52x

ypnklabs wrote on Saturday, March 18, 2017:

/* Define to trap errors during development. */
#if defined(DEBUG_NRF) || defined(DEBUG_NRF_USER)
#define configASSERT( x )                                                         ASSERT(x)
#endif
/* Hook function related definitions. */
#define configUSE_IDLE_HOOK 1
#define configUSE_TICK_HOOK  0
#define configCHECK_FOR_STACK_OVERFLOW       0
#define configUSE_MALLOC_FAILED_HOOK  0

ypnklabs wrote on Saturday, March 18, 2017:

Here is my idle hook code:

static void logger_thread(void * arg)
{
    while(1)
    {
        NRF_LOG_FLUSH();

        vTaskSuspend(NULL); // Suspend myself
    }
}
void vApplicationIdleHook( void )
{
     vTaskResume(m_logger_thread);
}

ypnklabs wrote on Saturday, March 18, 2017:

The third party code should be only doing arm math functions.

rtel wrote on Saturday, March 18, 2017:


You are not using a stack overflow hook.

tlafleur wrote on Saturday, March 18, 2017:

You should have all of these hook functions on for debugging… it will save you lots of headache…
Also, math function in general are stack hogs… so make sure your 3rd party task has lots of stack… do you know if it’s rtos safe?

~~ _/) _/) _/) ``` _/) ~~

Tom Lafleur

On Mar 18, 2017, at 9:12 AM, yvan pearson ypnklabs@users.sf.net wrote:

/* Define to trap errors during development. /
#if defined(DEBUG_NRF) || defined(DEBUG_NRF_USER)
#define configASSERT( x ) ASSERT(x)
#endif
/
Hook function related definitions. */
#define configUSE_IDLE_HOOK 1
#define configUSE_TICK_HOOK 0
#define configCHECK_FOR_STACK_OVERFLOW 0
#define configUSE_MALLOC_FAILED_HOOK 0
xSemaphoreTake --> crash, xSemaphoreTakeFromISR–>no crash

Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/freertos/discussion/382005/

To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/

ypnklabs wrote on Saturday, March 18, 2017:

Nice catch, but same problem…Take doesn’t work =(

// Need to figure out why *FromISR fixes the crash
            // while (pdFALSE == xSemaphoreTakeFromISR(m_sem_samples_event_ready, &pxHigherPriorityTaskWoken))
            while (pdFALSE == xSemaphoreTake(m_sem_samples_event_ready, portMAX_DELAY));

ypnklabs wrote on Saturday, March 18, 2017:

@Real Time Engineers ltd.

I’ll setup the hook and report back.

ypnklabs wrote on Sunday, March 19, 2017:

Hello,

Step1
I tried the stackoverflow hook. Wow, I love it =) I tested it to make sure it would be called when I took the code that was working and reduced the stack size of the thread in question and saw the hook got called. Then I returned the stack size to the original size.

Step2
I changed the “working code” Take() from (this is the consumer foreground thread doing the math on the ADC samples once the ISR copied the data to the static global buffer)

while (pdFALSE == xSemaphoreTakeFromISR(m_sem_samples_event_ready, &pxHigherPriorityTaskWoken))
to
while (pdFALSE == xSemaphoreTake(m_sem_samples_event_ready, portMAX_DELAY));

Result
The OS seems to have crash and the hook never gets called. Any other available options?

BTW thanks for your responses this weekend =)

richard_damon wrote on Sunday, March 19, 2017:

Your code still has the fundamental problem that when you call the FromISR in the task, NONE of your lower priority tasks, like Idle, and thus also your logger, never will get called.This says that the crash could easily be in one of those instead.

If you changet the xSemaphoreTake to use a delay of 0 and the same sort of busy spin loop while it is, do you get a similar result? If so then the problem could well be in any of the tasks held off by the busy spin loop.

You could also check this by adding a vTaskDelay(1) inside the spin loop of the TakeFromISR loop. (This will delay processing of the give, but will let the other tasks run),

There is also a possible issue that if the code being called in the ISR uses the floating point processor, in some ports these are not saved in the interrupt context (to save time as they are rarely used in ISRs) and thus breaking some non-interrupt code that is using floating point.

ypnklabs wrote on Sunday, March 19, 2017:

The following is the makefile used to make the third-party library in question.

CROSS=arm-none-eabi-
TEMPLATE_PATH = ../../components/toolchain/gcc
include $(TEMPLATE_PATH)/Makefile.posix
CC   = ${CROSS}gcc
STRIP= ${CROSS}strip
LD   = ${CROSS}ld
AS   = ${CROSS}as

TARGET = ba_lib.a
SFLAGS =  --strip-debug
CFLAGS += -c -mcpu=cortex-m4 $(CPATH) 
CFLAGS += -mthumb -mabi=aapcs
CFLAGS += -Wall -Werror -O1
CFLAGS += -mfloat-abi=hard -mfpu=fpv4-sp-d16
CFLAGS += -ffunction-sections -fdata-sections -fno-strict-aliasing
CFLAGS += -fno-builtin --short-enums
CFLAGS += -DARM_MATH_CM4
CFLAGS += -D__FPU_PRESENT=1 
CFLAGS += -D__MICROLIB
LDFLAGS=
SRCFOLDER=../
# Source Files
CSRC = cd_lib.c sr_pro.c demod.c 
COBJ=$(CSRC:.c=.o)
CSRC:=$(addprefix $(SRCFOLDER)/, $(CSRC))

#includes common to all targets
CPATH += -I$(abspath ../../components/toolchain/cmsis/include)
CPATH += -I$(abspath ../../components/toolchain/gcc)
CPATH += -I$(abspath ../../components/toolchain)

all: clean $(TARGET)

$(TARGET): $(COBJ)
	ar rc $@ $^ && ranlib $@

%.o: %.c
	$(CC) $(CFLAGS) $< -o $@
	$(STRIP) $(SFLAGS) $@

clean:
	@rm -f $(SRCFOLDER)*.o

ypnklabs wrote on Sunday, March 19, 2017:

Recall that if I comment out the third party lib the xSemaphoreTake() seems to work ok

ypnklabs wrote on Sunday, March 19, 2017:

Test 1:
Huh?
while (pdFALSE == xSemaphoreTake(m_sem_samples_event_ready, 0));
This appears to stop the crash…more updates to come

while (pdFALSE == xSemaphoreTake(m_sem_samples_event_ready, 1));
This seems to work too, but anything higher than 1 then the crash happens

The 3rd party library (using math) is only used in the thread context, not in the ISR.

I’m not familar with using the FPU which I’m sure the library uses. Would it use ISRs to do the calcs?