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

Fix reserveHT #1013

Merged
merged 1 commit into from
Dec 4, 2021
Merged

Conversation

splhack
Copy link
Contributor

@splhack splhack commented Aug 16, 2021

The nature of reserveHT is creating a CPU-list for taskset -c, it should work without Hyper-threading.
But it assumes hyperthreadingMultiplier is 2.
This PR allows non-HT machine (for example, VM) can use taskset -c.

(Probably, we can add an extra logic for hyperthreadingMultiplier=3 or more, but I think not worth it for now)

@bcipriano
Copy link
Collaborator

The change LGTM -- could you explain more what led you to make this change? I think I see the benefit of always using taskset, but I'd like to understand better.

@splhack
Copy link
Contributor Author

splhack commented Sep 12, 2021

The problem is this if logic in reserveHT to use taskset.

if hyperthreadingMultiplier > 1:

Current generation CPU cores' hyperthreadingMultiplier is always 2.
But it is not always true for VMs, etc.

@bcipriano
Copy link
Collaborator

What happens if taskset is not used?

@splhack
Copy link
Contributor Author

splhack commented Sep 12, 2021

taskset is here to launch the job command on the specified CPU lists by the CPU affinity.
Linux kernel schedules the process to run on any CPU without it.
It means RQD will lose the OpenCue concept, splitting machine into multiple computing resources for concurrent multiple job(frame, process.)

@splhack
Copy link
Contributor Author

splhack commented Nov 4, 2021

@bcipriano please let me know if anything I should clarify to merge.

@bcipriano bcipriano merged commit d4b790f into AcademySoftwareFoundation:master Dec 4, 2021
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.

2 participants