x86: Guard clock frequency against a divide by 0
ClosedPublic
Actions

Authored by ziaee on Tue, Mar 24, 4:47 AM.

Details

Reviewers

kib
markj
olce
obiwac
jrtc27
imp

Commits

rGc505fc146884: x86: Guard clock frequency against a divide by 0

Summary

We may be running in a Virtual Machine which may not fully support
hardware performance counters. If the MPERF counter somehow ends up
at zero, return an error and fail gracefully instead of panicing.

Sponsored by: Google
Suggested by: Jim Mattson <jmattson@google.com>
Suggested by: jrtc27 (split out this part)

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ziaee created this revision.Tue, Mar 24, 4:47 AM

Herald added a subscriber: imp. · View Herald TranscriptTue, Mar 24, 4:47 AM

ziaee requested review of this revision.Tue, Mar 24, 4:47 AM

Harbormaster completed remote builds in B71679: Diff 174189.Tue, Mar 24, 4:47 AM

ziaee added a parent revision: D55996: x86: Handle when MPERF/APERF MSRs aren't writable.Tue, Mar 24, 4:48 AM

obiwac accepted this revision.Tue, Mar 24, 5:42 AM

This revision is now accepted and ready to land.Tue, Mar 24, 5:42 AM

i think your stack is the wrong way around, right?

Fine.

In D56056#1282082, @obiwac wrote:

i think your stack is the wrong way around, right?

Seems so. :-) Be careful when you re-order things.

AustinWise_gmail.com added a subscriber: AustinWise_gmail.com.Thu, Mar 26, 12:00 AM

AustinWise_gmail.com added inline comments.

sys/x86/x86/cpu_machdep.c
466	Earlier in this function there is a call to `sched_bind` and at the end there is a call `sched_unbind`. Should this thread be unbound from the CPU when it exists through this error path?

obiwac added inline comments.Thu, Mar 26, 2:44 AM

sys/x86/x86/cpu_machdep.c
466	good catch ziaee@, I can update the patch for you to do this if you want

ziaee added inline comments.Thu, Mar 26, 1:07 PM

sys/x86/x86/cpu_machdep.c
466	That's very kind of you, yes please. Although, please don't commit it yet because I have to manually override the author. I am proxying this patch on behalf of Matt Delco, I forgot that Phabricator squashes author and committer when I uploaded it.

Proper cleanup when mcnt = 0

This revision now requires review to proceed.Thu, Mar 26, 1:32 PM

Harbormaster completed remote builds in B71721: Diff 174321.Thu, Mar 26, 1:32 PM

obiwac accepted this revision.Thu, Mar 26, 1:34 PM

obiwac added inline comments.

sys/x86/x86/cpu_machdep.c
466	you should probably add this as a note in your revision description then, so whoever commits this (can be me if no one has further objections) doesn't forget to update the author

This revision is now accepted and ready to land.Thu, Mar 26, 1:34 PM

This feels like it needs a comment somehow, but the earlier comment about APERF/MPERF seem to cover it well enough...

Closed by commit rGc505fc146884: x86: Guard clock frequency against a divide by 0 (authored by Matt Delco <delco@google.com>, committed by ziaee). · Explain WhyThu, Mar 26, 7:03 PM

This revision was automatically updated to reflect the committed changes.

ziaee added a commit: rGc505fc146884: x86: Guard clock frequency against a divide by 0.