Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOC] Documentation of non-obvious behaviour of mutex priority dis-inheritance #1168

Open
Hawk777 opened this issue Feb 1, 2024 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@Hawk777
Copy link

Hawk777 commented Feb 1, 2024

Describe the issue
FreeRTOS mutexes do priority inheritance. However, they do only somewhat rough priority disinheritance when releasing. In particular, xSemaphoreGive is a macro which calls xQueueGenericSend, which calls prvCopyDataToQueue, which calls xTaskPriorityDisinherit, which only disinherits priority if no other mutexes are held. It makes sense to do this (maintaining a data structure that would let you figure out the proper priority to disinherit to based on the current waiters on the remaining held mutexes would be complicated and expensive), but it’s not at all obvious that a priority-inheriting mutex would do this (the intuitive behaviour would be that the giver’s priority drops to the maximum of all waiters for all still-held mutexes), and as far as I can tell, it’s not documented in the manual. I think it should be, because it sometimes matters (it caused a difficult-to-find bug in my code, where I expected “give mutex immediately followed by take mutex” to result in the mutex being handed off to a task with a higher base priority, but it wasn’t actually handed off because the yielder held another irrelevant mutex at the same time which prevented its priority from being disinherited and therefore prevented a task switch from happening).

Reference
Priority inheritance is primarily documented on this page, so I think that’s where it would make sense to describe this. Maybe also here, here, here, and here, which also mention priority inheritance in a shorter form.

@Hawk777 Hawk777 added the documentation Improvements or additions to documentation label Feb 1, 2024
@paulbartell
Copy link
Member

@Hawk777 Thanks for the in-depth description. I'll work with our docs team to get this updated.

@kstribrnAmzn
Copy link
Member

kstribrnAmzn commented Apr 24, 2024

I've been taking a look through the code and found another unexpected disinheritance behavior here which is called when a task waiting for the mutex completes its wait for the mutex (or times out). In this case the priority once again will not be reset when more than one mutex is held.

I'll be working on a write up of this issue to give to our documentation team. I'm hoping to get this updated within the next week or two (our documentation receives updates weekly).

@Hawk777
Copy link
Author

Hawk777 commented Apr 25, 2024

@kstribrnAmzn good catch, that is a similar issue, it looks like when a waiter times out, the holder’s priority drops to the highest of any remaining waiter for that mutex, when in an ideal world it should drop to the highest of any remaining waiter for any mutex it holds, not just the one for which the timeout occurred.

@kstribrnAmzn
Copy link
Member

I think the behavior is slightly different from what you've described (and even more basic). When there are more than 1 mutexes still held, the priority does not get disinherited.

It gets calculated to be disinherited here, but because more than one mutex is held the tasks priority will not be updated by this line. This is tested by the test_vTaskPriorityDisinheritAfterTimeout_success3 unit test found here. In this test even though it could 'disinherit' to a higher priority it won't because the more than one mutex is held.

@kstribrnAmzn
Copy link
Member

The kernel book is updated. I'm working with our doc writer to get the website updated in the next week or so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants