Question about vTaskPrioritySet with mutex enabled

When mutex is enabled, in the function vTaskPrioritySet it says :

Only change the priority being used if the task is not currently using an inherited priority.

My question is what if the new priority being set is bigger than the priority inherited ? Shouldn’t we use the new priority?

To my understanding, the problem here is that FreeRTOS may in this scenario assume that the inherited priority is the task’s regular priority. You may want to query the forum for “priority inheritance,” FreeRTOS’ implementation of it is a tradeoff between performance and robustness issues, implying that it does not always behave as expected.

In either case, manipulating the priority of a task that already has a manipulated priority is something that bears many pitfalls regardless of the architecture of the priority inheritance architecture, so it is best to avoid it altogether (ie do not change the priority of a task that owns a mutex).

Thank you for your reply.

In my understanding, the trade-off of mutex is about a task can’t disinherit priority properly when the task has more than one mutex, because it is difficult to find the properly priority.

For vTaskPrioritySet, I think there are three situations:

  1. The task doesn’t hold a mutex, or the mutex doesn’t inherit priority yet.

  2. The task has inherit priority from mutex, and we are setting a new priority bigger than the inherited priority.

  3. The task has inherit priority from mutex, and we are setting a new priority not bigger than the inherited priority.

For situation 1, we can directly change task’s running priority;
for situation 3, we shoudn’t change task’s running priority;
for situation 2, I think we should change the task’s running priority to the bigger priority.

I believe that this is only one part of the priority inheritance issue. If you look at tasks.c, you will find that the TCB has a member called uxBasePriority which is used for bookkeeping of priority inheritance/disinheritance, but that also happens to be the member variable that vTaskPrioritySet() parties on, so (from superficial code analysis, I might be wrong) changing uxBasePriority via vTaskPrioritySet while it has been manipulated by priority inheritance may confuse the disinheritance mechanism regardless of whether the task holds one or multiple muteces OR the requested priority is higher or lower than the current priority.

In any case, again, I do not see a compelling use case for what you propose. The very idea of priority inheritance is that the OS helps establishing and maintaining real time behavior, so fiddling with that on the application side may compromise the determinism of the mechanism.

changing uxBasePriority via vTaskPrioritySet while it has been manipulated by priority inheritance may confuse the disinheritance mechanism

I don’t think so, vTaskPrioritySet has already changed uxBasePriority no matter the task holds mutex or not. See here.
The question is should we change the running priority uxPriority when new priority is higher than the inherited running priority.

In any case, again, I do not see a compelling use case for what you propose.

The use case is a task may call vTaskPrioritySet when the task holds a mutex, I don’t think this use case is forbidden, and the kernel should deal this situation properly.

I will not argue with you what “proper” is. If you feel that that there is a fault either in the code or the documentation, may I suggest you submit a PR or a doc change request, and the community will decide what the “proper” way to handle this should be. Thanks for contributing!

Looking at the existing code - I think implementing case 2 (from a few posts up) is worthwhile - need to check for the case that requires a context switch (as is normal any time you raise the priority of another task). Grateful for a PR for this.

I have submit a PR at here.

need to check for the case that requires a context switch (as is normal any time you raise the priority of another task)

@rtel I think the xYieldRequired and xYieldForTask in the existing code is enough for the context switch requirement.