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

Release core reminders for isolate threads #511

Merged
merged 1 commit into from
Jul 13, 2022

Conversation

liranr23
Copy link
Member

@liranr23 liranr23 commented Jul 4, 2022

We now release using unPin the VdsCpuUnit we need. This is good for
dedicated policy. When we are handling isolate-threads policy we
need to release the whole core. Otherwise the engine thinks it's taken.

Change-Id: I0b028e0dc5db72f5ef3099059f355d975a81fc90
Bug-Url: https://bugzilla.redhat.com/2103620
Signed-off-by: Liran Rotenberg lrotenbe@redhat.com

@ahadas
Copy link
Member

ahadas commented Jul 4, 2022

lgtm

@smelamud
Copy link
Member

smelamud commented Jul 4, 2022

In the case of isolate-threads policy you remove coresReminder * vm.getThreadsPerCpu() cores. @liranr23, is that what you wanted to do?

@liranr23
Copy link
Member Author

liranr23 commented Jul 4, 2022

In the case of isolate-threads policy you remove coresReminder * vm.getThreadsPerCpu() cores. @liranr23, is that what you wanted to do?

it does sounds to me we need to consider only the coreReminder (it's virtual) in isolate-threads, as each thread is taking a core (physical) and we do release now the whole core (including it threads - again physical). thanks!

@liranr23
Copy link
Member Author

liranr23 commented Jul 4, 2022

/ost

@ahadas
Copy link
Member

ahadas commented Jul 5, 2022

good catch @smelamud
@liranr23 how about adding a unit test?

@liranr23
Copy link
Member Author

liranr23 commented Jul 5, 2022

good catch @smelamud @liranr23 how about adding a unit test?

done with the bug case, with 1 core less in the test :)

We now release using unPin the VdsCpuUnit we need. This is good for
`dedicated` policy. When we are handling `isolate-threads` policy we
need to release the whole core. Otherwise the engine thinks it's taken.

Change-Id: I0b028e0dc5db72f5ef3099059f355d975a81fc90
Bug-Url: https://bugzilla.redhat.com/2103620
Signed-off-by: Liran Rotenberg <lrotenbe@redhat.com>
@mz-pdm
Copy link
Member

mz-pdm commented Jul 12, 2022

Good, looks nicer now!

@liranr23
Copy link
Member Author

/ost

@ahadas ahadas merged commit cfe7f48 into oVirt:master Jul 13, 2022
@liranr23 liranr23 deleted the release_isolated branch July 13, 2022 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants