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

Added CPU isolation #378

Merged
merged 38 commits into from
Sep 6, 2018
Merged

Added CPU isolation #378

merged 38 commits into from
Sep 6, 2018

Conversation

bergware
Copy link
Contributor

@bergware bergware commented Sep 4, 2018

Both CPU pinning and CPU isolation is possible thru the GUI

@Squidly271
Copy link
Contributor

Without adding this PR, if you isolate a core, will the docker pinning then disallow the isolated cores? Pinning a container to one or more isolated cores will effectively only allow execution on the first selected core.

@bergware
Copy link
Contributor Author

bergware commented Sep 5, 2018

The current implementation doesn't have restrictions.
I am no expert in this field, but why would you make isolated cores for containers?
Isn't there a conflict situation, because containers run under the host which has these cores excluded?

@Squidly271
Copy link
Contributor

Squidly271 commented Sep 5, 2018

The current implementation doesn't have restrictions.
I am no expert in this field, but why would you make isolated cores for containers?
Isn't there a conflict situation, because containers run under the host which has these cores excluded?

Not a conflict per se. What isolating CPUs actually does is stop the context switcher from utilizing those cores. Doesn't actually prevent any executable from manually utilizing them via the appropriate commands (ie: pinning a container to an isolated core) But, since the context switcher won't operate on those isolated cores, Linux won't switch the threads back and forth between isolated cores automatically. Net result is that if a container is pinned to multiple isolated cores, it will only actually execute on the first core. Hence why I would like to see that if the user is isolating cores then the cores become unavailable to pin to a container.

Would just solve some problems on the forum by removing the option to pin a container to an isolated core. (Plus it makes no sense anyways, because even without knowing the fundamental goings on beneath the surface, by isolating cores you are telling the OS to not use the cores for anything, but to reserve them for VMs)

Of course the complication arises if a user has already pinned cores, and then chooses to isolate them. But, that would be a whole different story catching that situation at time of docker run and adjusting the pinning accordingly. Probably something more suited for FCP to catch.

Side related thought: is there a variable (or can there be) that shows the mode unRaid booted in (GUI / normal)

@limetech
Copy link
Contributor

limetech commented Sep 5, 2018

The cephalopod is correct: pinning multiple isolated CPU's to a container only lets container utilize lowest numbered CPU. This is a kernel "issue" though kernel guys would probably argue it's a kernel "feature".

I think the best approach is to prohibit users from selecting isolated cores for container cpu-pinning. If user really wants to dedicate a single core to a container (maybe they turned off hyperthreading or CPU does not have hyperthreading), then they can turn off isolcpus, pin their cores the way they want, then turn isolcpus on the way they want. FCP might gripe but they'll know why it's griping.

re: boot mode: you could check for existence of /root/.mozilla/ or /root/.Xauthority/ which are only present in GUI boot mode.

Edit: could also grep for '/bzroot-gui' on /proc/cmdline

@bergware
Copy link
Contributor Author

bergware commented Sep 5, 2018

Adding restrictions may lead to confusion?
Perhaps we leave the assignments for the moment unrestricted and wait for user feedback.
I believe people also need to educate themselves when "toying" with this.

The help text can point people in the right direction. So maybe some of the explanation given here should go into the help text?

@limetech
Copy link
Contributor

limetech commented Sep 5, 2018

Yes that's fine.

@limetech limetech merged commit 53111db into unraid:master Sep 6, 2018
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.

3 participants