Best practice using mutex

Hi,

In my code, I have a lot of shared data protected by mutex. I had a deadlock problem because I used several mutex to protect one line of command. Here is a simple example:

xSemaphoreTake(MotherboardSwitches_mutex, portMAX_DELAY);
xSemaphoreTake(MotherboardSwitchesRequest_mutex, portMAX_DELAY);
FluidicsONRequest = !FluidicsON;
xSemaphoreGive(MotherboardSwitchesRequest_mutex);
xSemaphoreGive(MotherboardSwitches_mutex);

Now, let’s imagine that, in another task, we take MotherboardSwitchesRequest_mutex before MotherboardSwitches_mutex, there is some chance that it creates a deadlock.

To remove this possibility, I was thinking of using function to set/get the data. For example:

bool get_FluidicsON()
{
    xSemaphoreTake(MotherboardSwitches_mutex, portMAX_DELAY);
    bool result = FluidicsON;
    xSemaphoreGive(MotherboardSwitches_mutex);
}

That way, the previous example become:
set_FluidicsONRequest(!get_FluidicsON());

But if I do something like this, the code will call the xSemaphoreTake and xSemaphoreGive a lot. Is this a problem? Do you see a better way to fix my problem? One other way could be to create temporary copies of these variable that are protected, but the program become not really readable.

1,All global variables are the same style resource. so you could just use one mutex for all of them.
2,think over these variables, if some of them just be used only in two task,for example,one producer and one comsumer,so no protect needed.
3,try your best ,not use global variable but local. use other method instead.

Access a variable that can be accessed atomically (in a single instruction) doesn’t need protection, provided that access doesn’t need to be synchronized with another access.

Simple accessing of a variable that can’t be accessed atomically (perhaps too big, or you need something like an increment), can often be best done with a critical section rather than a mutex. My general rule is that I will use the critical section if the time in the section will be strictly bounded and ‘very short’ relative to other times in the system.

Looking at the operations needed in your program, this can greatly reduce the number of needed mutex, if many of the things you are protecting are simple enough not to need them.

When you do use mutexes, one VERY important rule is that every sequence of locking MUST use the same order. If one sequence locks mutexA before mutexB, then another sequence can NOT lock mutexB then mutexA. This is needed to avoid deadlock. One technique that is sometimes used is to assign every mutex a unique number, and every mutex take verifies that the number of the mutex you are taking is higher than any other mutex that you currently hold (in that Task). This will detect possible deadlocks without needing them to actually happen.

1 Like

+1, mutex is expensive. If have to use a global variable, could take a look at atomic.h.

Thank you for all your replies, I will try to reduce the number of mutex.
Is there an easy way to know if a variable will be accessed atomically or do I need to check the assembly code ?

What do you call “very short”? Would you call that example short?

for (int i = 0; i < 16; i++)
{
	for (int x = 0; x < 8; x++)
	{
	    PacketDataToSend[x] = DataToCAN[x + i * 8];
	}
	can_async_write(&CAN_0, &ReplyToCAN);
}

I don’t think C language (before C11) itself guarantees atomic access to memory. Atomic access to memory will either need underlying architectural support or a lower level software lock.

e.g. lower level software lock – what we have in atomic.h as of now is disabling interrupt globally.
e.g. architectural support – load-link/store-conditional Load-link/store-conditional - Wikipedia (ARM, RISC-V, … took this approach). Other architecture may do things slightly differently, and some may have ABA problem ABA problem - Wikipedia (Xtensa, if I remember correctly).

C compiler may offer extension for atomic memory access.
e.g. GCC Legacy __sync built-in __sync Builtins (Using the GNU Compiler Collection (GCC)).
e.g. Newer GCC __atomic built-in, memory model aware __atomic Builtins (Using the GNU Compiler Collection (GCC)).
Other compilers may have something similar. (I do remember seeing something in IAR and KEIL.)

But back to application code… It may be easier to review the structure of the code, what needs to be threaded and what needs not. (e.g. In power on initialization, the code is likely not thread safe but it’s likely intended and fine.) Though in your example, I think you meant to have “a global state being updated by probably ISR, and consumed by other threads”. Sounds some protection is needed…

For Atomic accesses, generally I know the type of processor I am targeting an application to, so I know what size the processor can naturally read or write in a single cycle, and if the processor can, then unless you are doing something strange, the compiler generally will use that instruction.

‘Very Short’ is somewhat application dependent. The first question you have to ask yourself is how long can you allow a critical section, and still meet whatever interrupt latency requirements you have. The biggest cost of a critical section is to interrupt latency. Assume that right after the critical section starts, all the possible timing critical interrupts fire, do you still meet those timing requirements with a safety margin?

Your code above is copying 128 elements AND making 16 calls to a function. First glance, that doesn’t sound very short, but depending on the system, it might be able to be argued it is (if can_async_write is a fairly trivial function, maybe even inlined)

I am going to presume that this is collecting a bunch of data from multiple processes, and when it is all collected, it is sent and then the collection begins again. If that is the case, then you don’t need a mutex as the natural sequence will enforce things. This task won’t access it until everyone filling says they are done, and the fillers won’t refill until they are notified that the data has been grabbed.

If some of the fillers might want to update what the previously posted before it is sent, you may need the mutex (or the fillers, sending a much smaller chunk of data use a critical section AND be lower in priority then the sender).

Ok, thank you. I’m targeting an ATSAMV71 board so I will try to find information about how Cortex M7 handle data.

I understood the variable size problem and that a 64 bit variable can be read as two 32-bit numbers.
Although, for small variable, I’m not sure of something. Let’s take the example where we read a variable “status”:
uint8_t StateCopy = State;
The compiler translates this instruction with:
0040EF10 ldr r3, [pc, #664]
0040EF12 ldrb r3, [r3]
0040EF14 strb r3, [r7, #28]
Is it an atomic access to the variable or not? The first line load the variable address, then we read the variable stored at this address, finally we store this value into our new variable.
If I understood, the variable memory is only used on the second line, so I guess that it’s an atomic access. Although, what happens if the processor execute the first instruction (load the variable address), then switch to another task and finally comeback to execute the 2 other instructions ? Is there a chance that the variable address stored in r3 has been erased ?

The access is atomic, as only a single instruction is used to actually read the variable (ldrb r3,[r3]). The M7 core can read or write a single 8, 16 or 32 bit object like this.

Where the system switches between tasks, it needs to save and restore ALL the registers, if the program can’t count on a register holding its value between two access, then it has a hard time running correctly.

Now, if the variable was being accessed through a pointer that might have changed, then you could have that issue, you load the pointer address, switch out, the pointer gets changed, so now you access some dead storage, so access through pointers that can change isn’t atomic.

1 Like

Thanks a lot for your answers, it helps me a lot. I will try to get ride of all useless mutex of my code and try to reduce as much as I can the number of global variables that I use.

One thing to note, it isn’t that the variables are global that means you need to protect them, it is that they are shared between tasks, you get that same problem if the variables are local but their address is shared to multiple tasks or an access function for them is used by multiple tasks. Global just make it easier to do this, and maybe make the multiple access not as obvious.

You need to think about how data is shared between tasks