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

Reduce real-time priority to 98 #3

Merged
merged 2 commits into from
Oct 31, 2022
Merged

Reduce real-time priority to 98 #3

merged 2 commits into from
Oct 31, 2022

Conversation

simutisernestas
Copy link
Contributor

@simutisernestas simutisernestas commented Oct 31, 2022

was reading upon it, here seems like it's a bad idea to set RT priority to max 99 : )

@2b-t 2b-t changed the title Reduce prior group by one Reduce real-time priority to 98 Oct 31, 2022
@2b-t
Copy link
Owner

2b-t commented Oct 31, 2022

Hi @simutisernestas ,
Thanks for your merge request.
I have heard as well that you should not put your thread's real-time priority to 99 and instead leave it below the priority of kernel threads (some say 98, others say 90-95 or even lower values depending on your application).
Yet the rtprio set there is only the maximum realtime priority allowed inside the container. It is what you see when you type $ ulimit -a inside it and you are free to set any priority within these limits for your user threads, e.g. with chrt --rr 98 <command>. While I agree that for most people running their threads with an rtprio of 99 is likely not what they want, I do not know if there are not cases where setting such an rtprio might actually be desirable. Therefore I passed this responsibility on to the user inside the container, so they might still decide if it is something they want to use or not. This seems to be what the Docker of the pendulum demo does with the command --ulimit rtprio=100:100 as well.
While I am not sure if this is something that should be changed, I agree that at least a comment pointing this out somewhere might be beneficial for somebody not familiar with the topic.

@simutisernestas
Copy link
Contributor Author

I've reverted to 99, but left the comment in case you wish to keep it :)

@2b-t 2b-t merged commit 9f28842 into 2b-t:main Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants