Hi

Port PIC24, ver, 5.20
Compilator: pic30-gcc v3.12 from microchip, opt. flag O2 + some minors flags


Disassembly listing for vTaskSuspendAll()
--------------------------------------------------------------------------------------------------------
1015:
1016: void vTaskSuspendAll( void )
1017: {
1018: /* A critical section is not required as the variable is of type
1019: portBASE_TYPE. */
1020: ++uxSchedulerSuspended;
1399A 81D890 mov.w 0x3b12,0x0000
1399C E80000 inc.w 0x0000,0x0000
1399E 89D890 mov.w 0x0000,0x3b12
1021: }
139A0 060000 return
--------------------------------------------------------------------------------------------------------

So it takes 3 instruction: load uxSchedulerSuspended to WREG0, increment WREG0 and store results.
So I do not see reason for comment:
/* A critical section is not required as the variable is of type
portBASE_TYPE. */

Please comment.
Best Regards
/Greg

That does not look right!

That is a very recent change, and I agree it looks wrong. I will investigate. There will be a new release on the 22nd so it can get fixed quickly.

Thanks for pointing this out.

Regards.

Actually, I think it is ok, let me explain.

If uxSchedulerSuspended starts off non-zero, then we don't need a critical section, as an interrupt during the execution of the code will not change the thread of execution, so the non-atomic sequence will execute ok (a key fact is that no interrupt will change the value of this variable and not set it back to what it started as).

If uxSchedulerSuspended starts of zero, then it is possible for the thread of execution to be interrupted, but if we do execute a task switch, then when we get switched back, by definition the variable is zero again (if not, we couldn't have been been switched back to).

The key fact is that due to the nature of this variable, a task can not see it changed by another task, it will always be zero if something else can run (that task can change it, but must restore it before you get control back).

In fact I think it will work even if the read and writes to uxSchedulerSuspended aren't atomic in most practical situations (the most likely would be hitting a nesting depth suspensions of 256 on a machine that writes 1 byte at a time, then the counting from 255 to 256 might go 255->0(write low byte) -> 256 (write high byte) and if you get in interrupt for the moment when it is 0 you will have a problem.

Excellent. Thanks for the clarity.

I would like to say "that was exactly what I was thinking", but um...well its getting late.

Regards.

Hi

Hmmm.., yes, that's reasonable explanation. Of course based on assumption that read and write is atomic.

Regards
/Greg

Yes, the access to the count needs to be atomic (or at least atomic "enough", the bits that might be non-zero need to be accessed atomically, depending on how many layerd of vTaskSuspendAll() might be nested, that may just be the bottom couple of bits.