Quality RTOS & Embedded Software

 Real time embedded FreeRTOS RSS feed 
Quick Start Supported MCUs PDF Books Trace Tools Ecosystem


Loading

Should xPendingReadyList be volatile?

Posted by xelus on February 8, 2019

Hello, I am using FreeRTOS 0.9.0 and MSP430XMSP430FR5969 port, and I am compiling it with TI 18.1 compiler. I have found error, which causes device to hangs out. The error occurs when at least two task are inserted to xPendingReadyList in interrupts when scheduler is suspended. When xTaskResumeAll function is call the microntroller only once evaluate listGETOWNEROFHEAD_ENTRY macro value (before while loop), but this value is changed during moving list item to task ready list. If I change the definition xPendingReadyList on volatile it works correctly ~~~

define listGETOWNEROFHEADENTRY( pxList ) ( (&( ( pxList )->xListEnd ))->pxNext->pvOwner )

while( listLISTISEMPTY( &xPendingReadyList ) == pdFALSE ) { pxTCB = ( TCBt * ) listGETOWNEROFHEAD_ENTRY( ( &xPendingReadyList ) ); ( void ) uxListRemove( &( pxTCB->xEventListItem ) ); ( void ) uxListRemove( &( pxTCB->xStateListItem ) ); prvAddTaskToReadyList( pxTCB );

/* If the moved task has a priority higher than the current
task then a yield must be performed. */
if( pxTCB->uxPriority >= pxCurrentTCB->uxPriority )
{
    xYieldPending = pdTRUE;
}
else
{
    mtCOVERAGE_TEST_MARKER();
}

} ~~~

Should xPendingReadyList be volatile or it is compiler error?


Should xPendingReadyList be volatile?

Posted by rtel on February 8, 2019

Can you please try adding the following line to FreeRTOSConfig.h, then report back as to whether it solves the problem for you:

~~~

define configLIST_VOLATILE volatile

~~~


Should xPendingReadyList be volatile?

Posted by xelus on February 8, 2019

Thanks for tip, I was looking for such an option and somehow missed it. Unfortunatelly, adding volatile didn't help, and even got worse. Now device hangs after first tick, because of error in: vTaskSwitchContext ---> taskSELECTHIGHESTPRIORITYTASK ---> listGETOWNEROFNEXT_ENTRY ~~~

define listGETOWNEROFNEXTENTRY( pxTCB, pxList )

{ List_t * const pxConstList = ( pxList ); /* Increment the index to the next item and return the item, ensuring / / we don't return the marker used at the end of the list. */ ( pxConstList )->pxIndex = ( pxConstList )->pxIndex->pxNext; if( ( void * ) ( pxConstList )->pxIndex == ( void * ) &( ( pxConstList )->xListEnd ) ) { ( pxConstList )->pxIndex = ( pxConstList )->pxIndex->pxNext; } ( pxTCB ) = ( pxConstList )->pxIndex->pvOwner; } ~~~ In first line, pxIndex is overwritten by pxIndex->pxNext value. Next the address comparision is made, and at this point compiler gets new value of pxIndex. Because condition is met pxIndex is again overwritten by pxIndex->pxNext value, but this is the same value as it is in first line.

Asm code when volatile is enabled: ~~~ R14 is pxConstList 01f582: 0E3C 0002 MOVA 0x0002(R14),R12 ;R12 = ( pxConstList )->pxIndex 01f586: 1800 4CDE 0004 0002 MOVX.A 0x00004(R12),0x00002(R14) ;now pxIndex is updated 01f58e: 0ECF MOVA R14,R15 01f590: 00AF 0006 ADDA #0x00006,R15 01f594: 1800 9FCE 0002 CMPX.A R15,0x00002(R14) ;we using current ( pxConstList )->pxIndex value 01f59a: 2004 JNE (0xf5a4) 01f59c: 1800 4CDE 0004 0002 MOVX.A 0x00004(R12),0x00002(R14) ; at this point compiler are using old R12 value $C$L2869: 01f5a4: 0E3F 0002 MOVA 0x0002(R14),R15 01f5a8: 1800 4FD2 000C 3AE4 MOVX.A 0x0000c(R15),&pxCurrentTCB 01f5b0: 4D82 3AEE MOV.W R13,&uxTopReadyPriority 01f5b4: 0110 RETA
~~~


Should xPendingReadyList be volatile?

Posted by rtel on February 9, 2019

Ok - I need to get a better understanding of the issue you are facing. I will go back to your original post:

I am using FreeRTOS 0.9.0 and MSP430X_MSP430FR5969 port

I presume you mean FreeRTOS V9.0.0. Why use an old version?

and I am compiling it with TI 18.1 compiler.

Is that the latest compiler version?

I have found error, which causes device to hangs out. The error occurs when at least two task are inserted to xPendingReadyList in interrupts when scheduler is suspended. When xTaskResumeAll function is call the microntroller only once evaluate listGETOWNEROFHEADENTRY macro value (before while loop),

This bit I don't understand. I think you are referring to the function xTaskResumeAll(), which (for V9.0.0) can be seen here: https://sourceforge.net/p/freertos/code/HEAD/tree/tags/V9.0.0/FreeRTOS/Source/tasks.c#l2017

The code you pasted starts on line 2041 https://sourceforge.net/p/freertos/code/HEAD/tree/tags/V9.0.0/FreeRTOS/Source/tasks.c#l2041

....but where is listGETOWNEROFHEADENTRY() being called in that function before the while loop? I presume you mean it is being called INSIDE the while loop (line 2043).

You say listGETOWNEROFHEADENTRY() is only evaluated once, which I assume means the while() loop only executes once, which implies that, even though xPendingReadyList contains two tasks, the line:

~~~ while( listLISTISEMPTY( &xPendingReadyList ) == pdFALSE ) ~~~

returns pdFALSE when there are two tasks in the list, but doesn't return false when there is only one task in the list (one task having been removed during the first go around the loop).

Is that correct?

If I change the definition xPendingReadyList on volatile it works correctly

Can you please show how you updated the code to make the list volatile as my suggestion just made the members of the list volatile.


Should xPendingReadyList be volatile?

Posted by rtel on February 9, 2019

I just tried the following mocked up test, but I'm not sure I'm testing the right thing.

First I created a function that suspended the scheduler and manually added two tasks to the pending ready list:

void AddTasksToPendingReadyList( void );
void AddTasksToPendingReadyList( void )
{
TaskHandle_t x1, x2;
tskTCB *y1, *y2;

	/* Create two tasks, doesn't matter what they are. */
	xTaskCreate(	prvIdleTask,
					"X1", configMINIMAL_STACK_SIZE,
					( void * ) NULL,
					( tskIDLE_PRIORITY | portPRIVILEGE_BIT ),
					&x1 );

	xTaskCreate(	prvIdleTask,
					"X2", configMINIMAL_STACK_SIZE,
					( void * ) NULL,
					( tskIDLE_PRIORITY | portPRIVILEGE_BIT ),
					&x2 );


	y1 = ( tskTCB * ) x1;
	y2 = ( tskTCB * ) x2;

	/* Manually add the tasks to the pending ready list while the scheduler
	is suspended. */
	vTaskSuspendAll();
	vListInsertEnd( &( xPendingReadyList ), &( y1->xEventListItem ) );
	vListInsertEnd( &( xPendingReadyList ), &( y2->xEventListItem ) );
}

Then I created a function that printed the number of items in the pending ready list:

void CheckTasksInList( void );
void CheckTasksInList( void )
{
	printf( "Tasks in list: %d\r\n", ( int ) xPendingReadyList.uxNumberOfItems );
	fflush( stdout );
}

I then called the following sequence from the idle task:

   taskENTER_CRITICAL(); // Prevent context switches.
	CheckTasksInList(); // Should print 0 tasks in list.
	AddTasksToPendingReadyList(); // Adds two tasks while scheduler suspended. */
	CheckTasksInList(); // Should print 2 tasks in list.
	xTaskResumeAll();
	CheckTasksInList(); // Should print 0 tasks in list again.

And sure enough I observed the expected behaviour, even without volatile defined, and with GCC optimization set to its most aggressive:

Tasks in list: 0
Tasks in list: 2
Tasks in list: 0

I also stepped through the while loop in xTaskResumeAll() to ensure two iterations were observed.


Should xPendingReadyList be volatile?

Posted by ldb on February 9, 2019

The volatile is in the wrong place :-)

It is currently ListItemt * configLISTVOLATILE pxIndex;

try configLISTVOLATILE ListItemt * pxIndex;

(type * volatile ptr) means the thing at the pointer address is volatile (volatile type * ptr) means the pointer itself is volatile

The compiler is correctly asssuming the pointer doesn't change because it isn't volatile the thing at the pointer is volatile .. it is behaving exactly as it should.

If you ever get confused go for safety (volatile type * volatile ptr) Now the pointer and what it points to are both volatile :-)


Should xPendingReadyList be volatile?

Posted by xelus on February 9, 2019

Thanks for your help. > I presume you mean FreeRTOS V9.0.0. Why use an old version?

Yes, I'm using 9.0.0 version, and I'm using this old version because this is old code, which is only bugfixed. I'm not sure what happens, when I will change to the newest Free RTOS version...

Is that the latest compiler version?

Yes, I have used in test the newest version

..but where is listGETOWNEROFHEADENTRY() being called in that function before the while loop? I presume you mean it is being called INSIDE the while loop (line 2043).

This macro is called inside loop, but in assembler code value is evaluated into microcontroller register before loop begins. Without any volatiles it looks like: ~~~ 2041 while( listLISTISEMPTY( &xPendingReadyList ) == pdFALSE ) (1) 019e9c: 9382 3980 TST.W &xPendingReadyList (2) 019ea0: 2486 JEQ (0x9fae) (3) 019ea2: 0028 398A MOVA &0x0398a,R8 2043 pxTCB = ( TCBt * ) listGETOWNEROFHEADENTRY( ( &xPendingReadyList ) ); (4) 019ea6: 0839 000C MOVA 0x000c(R8),R9 (5) (.....) 2052 xYieldPending = pdTRUE; (6) 019f74: 4392 3AEA MOV.W #1,&xYieldPending 2041 while( listLISTIS_EMPTY( &xPendingReadyList ) == pdFALSE ) (7) 019f78: 9382 3980 TST.W &xPendingReadyList (8) 019f7c: 2394 JNE (0x9ea6) 2060 if( pxTCB != NULL ) ~~~ (1) - check if list isn't empty (2) - exit when list is empty (3) - put into R8 register ( pxList )->xListEnd ))->pxNext (pointer value) (4) - dereferencing ( (&( ( pxList )->xListEnd ))->pxNext->pvOwner (7) - check if list isn't empty (and it is not) (8) - jump to line (4) if there is another task left in the list. In the meantime ( pxList )->xListEnd ))->pxNext was changed, but do not recalculate R8 register value If there is only one task, then in line (8) it out of the loop and everything work fine.

Can you please show how you updated the code to make the list volatile as my suggestion just made the members of the list volatile. I added to FreeRTOSConfig.h line: ~~~

define configLIST_VOLATILE volatile

~~~

I added your test function to the code and anduring execution it enters into infinite loop - just like in my case. listGETOWNEROFHEADENTRY always return X1 task.

(type * volatile ptr) means the thing at the pointer address is volatile (volatile type * ptr) means the pointer itself is volatile

Isn't it inversly? (type * volatile ptr) means the pointer itself is volatile (volatile type * ptr) means the thing at the pointer address is volatile

The compiler is correctly asssuming the pointer doesn't change (...)

pxIndex is changed directly one line up, so I think, that compiler shouldn't assuming that it isn't changed, even if it isn't set as volatile

I have temporarily added a configLISTVOLATILE macro and changed the definitions: ~~~ /* * Definition of the only type of object that a list can contain. */ struct xLISTITEM { listFIRSTLISTITEMINTEGRITYCHECKVALUE /*< Set to a known value if configUSELISTDATAINTEGRITYCHECKBYTES is set to 1. / configLISTVOLATILE TickTypet xItemValue; /< The value being listed. In most cases this is used to sort the list in descending order. / struct xLISTITEM volatile * configLISTVOLATILE pxNext; /< Pointer to the next ListItemt in the list. */ struct xLISTITEM volatile * configLISTVOLATILE pxPrevious; /*< Pointer to the previous ListItemt in the list. / void * pvOwner; /< Pointer to the object (normally a TCB) that contains the list item. There is therefore a two way link between the object containing the list item and the list item itself. / void * configLIST_VOLATILE pvContainer; /< Pointer to the list in which this list item is placed (if any). / listSECONDLISTITEMINTEGRITYCHECK_VALUE /< Set to a known value if configUSELISTDATAINTEGRITYCHECKBYTES is set to 1. */ }; typedef volatile struct xLISTITEM ListItem_t; /* For some reason lint wants this as two separate definitions. */

struct xMINILISTITEM { listFIRSTLISTITEMINTEGRITYCHECKVALUE /*< Set to a known value if configUSELISTDATAINTEGRITYCHECKBYTES is set to 1. */ configLISTVOLATILE TickTypet xItemValue; struct xLISTITEM volatile * configLISTVOLATILE pxNext; struct xLISTITEM volatile * configLISTVOLATILE pxPrevious; }; typedef volatile struct xMINILISTITEM MiniListItem_t;

/* * Definition of the type of queue used by the scheduler. / typedef struct xLIST { listFIRSTLISTINTEGRITYCHECKVALUE /< Set to a known value if configUSELISTDATAINTEGRITYCHECKBYTES is set to 1. */ configLISTVOLATILE UBaseTypet uxNumberOfItems; ListItemt * configLISTVOLATILE pxIndex; /*< Used to walk through the list. Points to the last item returned by a call to listGETOWNEROFNEXTENTRY (). */ MiniListItemt xListEnd; /< List item that contains the maximum possible item value meaning it is always at the end of the list and is therefore used as a marker. */ listSECONDLISTINTEGRITYCHECKVALUE /< Set to a known value if configUSELISTDATAINTEGRITYCHECKBYTES is set to 1. */ } volatile Listt; ~~~ And now it's work ok in both situations - during switiching task and during handling two tasks in xPendingReadyList list. Assembler look like: ~~~ 2041 while( listLISTISEMPTY( &xPendingReadyList ) == pdFALSE ) (1) 019e70: 9382 3980 TST.W &xPendingReadyList (2) 019e74: 2488 JEQ (0x9f86) 2043 pxTCB = ( TCBt * ) listGETOWNEROFHEADENTRY( ( &xPendingReadyList ) ); (3) 019e76: 002F 398A MOVA &0x0398a,R15 (4) 019e7a: 0F39 000C MOVA 0x000c(R15),R9 (5) (...) 2052 xYieldPending = pdTRUE; 019f4c: 4392 3AEA MOV.W #1,&xYieldPending (6) 2041 while( listLISTIS_EMPTY( &xPendingReadyList ) == pdFALSE ) $C$L38: (7) 019f50: 9382 3980 TST.W &xPendingReadyList (8) 019f54: 2390 JNE (0x9e76) ~~~ Now in line (8) it jumps to line (3), where pointer value is evaluated ( pxList )->xListEnd ))->pxNext


Should xPendingReadyList be volatile?

Posted by rtel on February 10, 2019

@Leon - good debate, however.....

If, in the following case (cut and paste from compiler manual):

volatile int attncount;
volatile int * acptr;

"declare the object attncount to be an integer whose value may be 
altered at any time (say by an asynchronous attention handler), and the 
object acptr to be a pointer to a volatile object of integer type."

then are you sure:

(type volatile ptr) means the thing at the pointer address is volatile (volatile type ptr) means the pointer itself is volatile

is correct?

In the extract from the manual, 'volatile int * acptr' is in the format (volatile type ptr), and it says the opposite, that the pointer is pointing to a volatile object (not that the pointer itself is volatile).

Further, https://barrgroup.com/Embedded-Systems/How-To/C-Volatile-Keyword says the following two are equivalent:

volatile uint8_t * p_reg;
uint8_t volatile * p_reg;

One is in the format (volatile type ptr) and the other (type volatile ptr).

Also, in the case of xTaskResumeAll(), the code in question is inside a critical section, so no values can be altered outside of the compiler's knowledge (an interrupt can't change the value, another task can't change the value, and the value is not mapped to a hardware register that can change at any time) - so as the compiler can see all the code is the 'volatile' qualifier necessary at all?


Should xPendingReadyList be volatile?

Posted by rtel on February 10, 2019

My last post is a couple up from here, see https://sourceforge.net/p/freertos/discussion/382005/thread/4edb6fb4b4/#1f4d/ee7d


Should xPendingReadyList be volatile?

Posted by ldb on February 10, 2019

The guy who wrote the article said Volatile pointers to non-volatile data are very rare (I think I've used them once), but I'd better go ahead and give you the syntax:

In your post above you gave only 2 options there are actually 3 volatile uint8t * preg; uint8t volatile * preg; uint8t * volatile preg;

I should say that answer folows this https://stackoverflow.com/questions/9935190/why-is-a-point-to-volatile-pointer-like-volatile-int-p-useful

Which is fine except http://www.cs.utah.edu/~regehr/papers/emsoft08-preprint.pdf

However as per above I suspect 2 & 3 are compiler dependant I know a lot of embedded compilers that don't interpret it that way suggested as the standard. From his code output I would say the volatile is being silently ignored.

As even your post stated volatile in that form are rare and I would suggest best avoided. He fixed it by simply dragging it out and typecasting it a volatile pointer which makes it very clear to the compiler (every compiler unddrstands that one).


Should xPendingReadyList be volatile?

Posted by ldb on February 10, 2019

It works now because you have a proper volatile pointer that the compiler recognizes. As I said above I think it is a case of compiler silently dropping the volatile.


Should xPendingReadyList be volatile?

Posted by xelus on February 10, 2019

Also, in the case of xTaskResumeAll(), the code in question is inside a critical section, so no values can be altered outside of the compiler's knowledge (an interrupt can't change the value, another task can't change the value, and the value is not mapped to a hardware register that can change at any time) - so as the compiler can see all the code is the 'volatile' qualifier necessary at all?

Macro listGETOWNEROFHEADENTRY use pxList->xListEnd->pxNext pointer value, which is changed indirectly in uxListRemove function. Compiler doesn't know, where pvContainer points, and by the pvContainer pointer we change ( pxList )->xListEnd ))->pxNext value.

I made a test with pointers declarations.

~~~ int * volatile ptr;

volatile int *ptr1; volatile int * volatile ptr2;

int volatile *ptr3; int volatile * volatile ptr4;

volatile int volatile *ptr5; volatile int volatile * volatile ptr6; ~~~

The compiler reports warning for the ptr5 and ptr6. So it's treats volatile uint8t * preg; uint8t volatile * preg; as the same.

In my case, it looks like the compiler treats pxIndex as a pointer to volatile data, because it's read again from memory pxIndex->pvOwner in the second iteration. If the compiler would treat the pointer as volatile, it should also read pxIndex value from memory before dereference.


Should xPendingReadyList be volatile?

Posted by ldb on February 11, 2019

Yeah it is as I thought the compiler does not know how to make a volatile pointer with that syntax which is not unusual.

As it is always accessed from the pointer below make the pointer below volatile which is the normal format of volatile which every compiler understands.

~~~ struct xMINILISTITEM { listFIRSTLISTITEMINTEGRITYCHECKVALUE
configLISTVOLATILE TickTypet xItemValue; volatile struct xLISTITEM * pxNext; / * now a normal volatile ptr * / volatile struct xLISTITEM * pxPrevious; / * now a normal volatile ptr * / }; typedef volatile struct xMINILISTITEM MiniListItemt;

/* * Definition of the type of queue used by the scheduler. */ typedef struct xLIST { listFIRSTLISTINTEGRITYCHECKVALUE configLISTVOLATILE UBaseTypet uxNumberOfItems; volatile ListItemt * pxIndex; / * now a normal volaile pointer * / MiniListItemt xListEnd;
listSECONDLISTINTEGRITYCHECKVALUE
} volatile List_t; ~~~

Do you notice the inconsistancy above in one case it uses ListItem_t which is bizzarely marked volatile (on your compiler I doubt it does what they expect) and everywhere else they used a struct pointer.


[ Back to the top ]    [ About FreeRTOS ]    [ Privacy ]    [ Sitemap ]    [ ]


Copyright (C) Amazon Web Services, Inc. or its affiliates. All rights reserved.

Latest News

FreeRTOS v10.2.0 is available for immediate download. MIT licensed, and including RISC-V and ARMv8-M (Cortex-M33) demos.

NXP tweet showing LPC5500 (ARMv8-M Cortex-M33) running FreeRTOS.

View a recording of the "OTA Update Security and Reliability" webinar, presented by TI and AWS.


Careers

FreeRTOS and other embedded software careers at AWS.



FreeRTOS Partners

ARM Connected RTOS partner for all ARM microcontroller cores

Cadence Tensilica Cortes

Espressif ESP32

IAR Partner

Microchip Premier RTOS Partner

RTOS partner of NXP for all NXP ARM microcontrollers

Mediatek

Renesas

RISC-V

SiFIve RISC-V

STMicro RTOS partner supporting ARM7, ARM Cortex-M3, ARM Cortex-M4 and ARM Cortex-M0

Texas Instruments MCU Developer Network RTOS partner for ARM and MSP430 microcontrollers

OpenRTOS and SafeRTOS

Xilinx Microblaze and Zynq partner