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.

Sign up for GitHub

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

Jump to bottom

FreeBSD: scale CPU% usage on swap time #606

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

FreeBSD: scale CPU% usage on swap time #606

wants to merge 1 commit into from

Conversation

cgzones
Copy link
Member

@cgzones cgzones commented Apr 18, 2021

@cgzones cgzones mentioned this pull request Apr 18, 2021
Open
BenBE
BenBE reviewed Apr 18, 2021
View reviewed changes
freebsd/FreeBSDProcessList.c
if (sysctlbyname("kern.ccpu", &ccpu, &size, NULL, 0) == -1) {
ccpu = 0;
}
const double decayfactor = log(ccpu / kernelFScale);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kernelFScale might be zero. Not ensured non-zero by the code querying this from the syscall.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

htop/freebsd/FreeBSDProcessList.c

Lines 138 to 142 in feec16c

len = sizeof(kernelFScale);
if (sysctlbyname("kern.fscale", &kernelFScale, &len, NULL, 0) == -1) {
//sane default for kernel provided CPU percentage scaling, at least on x86 machines, in case this sysctl call failed
kernelFScale = 2048;
}

You mean kern.fscale might be 0? Is that possible?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highly unlikely, given that there's other code already that does a similar division and I'm not aware of any bug reports regarding this. Mostly DiD note.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, https://is.cuni.cz/webapps/zzp/download/120118301

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually just looking at the remarked upon code part: ccpu is set to 0, which will cause a math error with the following calculation for decayfactor. I don't think that's right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made pull request #1275 that would reject zero kern.fscale value.

@BenBE BenBE added this to the 3.0.6 milestone Apr 18, 2021
@BenBE BenBE added bug 🐛 Something isn't working FreeBSD 👹 FreeBSD related issues labels Apr 18, 2021
@BenBE BenBE modified the milestones: 3.1.0, 3.1.1 Jul 15, 2021
@cgzones cgzones added the needs-discussion 🤔 Changes need to be discussed and require consent label Aug 8, 2021
@fasterit fasterit modified the milestones: 3.2.0, 3.2.1 Apr 26, 2022
@BenBE BenBE modified the milestones: 3.2.1, 3.2.2 May 21, 2022
@BenBE BenBE modified the milestones: 3.2.2, 3.3.0 Jan 31, 2023
@BenBE BenBE modified the milestones: 3.3.0, 3.4.0 Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gaod gaod gaod left review comments

@BenBE BenBE BenBE left review comments

@Explorer09 Explorer09 Explorer09 left review comments

Assignees
No one assigned
Labels
bug 🐛 Something isn't working FreeBSD 👹 FreeBSD related issues needs-discussion 🤔 Changes need to be discussed and require consent
Projects
None yet
Milestone
3.4.0
Development

Successfully merging this pull request may close these issues.

Is the CPU% 4.3 right on FreeBSD?
5 participants
@cgzones @gaod @BenBE @Explorer09 @fasterit