fix cpu_openbsd.go once and for all #1244
Conversation
6.3 was EOL'd more than three years ago!
We can't use unix.Sysctl* for some sysctls, so we're on our own with converting data from C arrays. Don't assume that the byte order is little endian but do the right thing. Moreover, there's a little distinction in the sizes reported by KERN_CPTIME (long[cpustates]) and KERN_CPTIME2 (u_int64_t[cpustates]) so account for that too.
don't make assumptions on which CPUs are online and wich aren't based on hw.smt and hw.ncpuonline. Rather, use KERN_CPUSTATS to get the CPU statistics, which includes a flag field that can tell us if that CPU is online or not.
Even thought OpenBSD often breaks the ABI compatibility and doesn't make
*any* promise of "stability", this project aims to be "pure go" so avoid
doing inter-op at the cost of artificially reducing the number of
supported architectures down to amd64 and i386.
To add support for another architecture (e.g. arm), add another file
cpu_openbsd_${arch}.go like done for 386 and amd64. The fields are
declared as `long' in C, so pick the appropriate size when declaring the
struct.
|
OK, I've got rid of the cgo bits and polished it a bit. I've tested on amd64 -CURRENT (with both smt enabled and disabled) and on i386 -CURRENT. |
|
P.S.: I think I could have used |
|
happy to have been useful and apologize again for my initial mistake :) Cheers! |
|
Can you cut a new tag on this repo so that I can try it with Telegraf? |
Actually, I figured out how to build from this most recent commit and good news, telegraf works now! Thanks @omar-polo @shirou still, if you can cut a release or a tag, I will pester the telegraf folks to update the dep. Thanks! |
|
gopsutil has a monthly release policy. plz wait a few days. |
Hello,
as reported in #1241 currently cpu_openbsd.go doesn't retrieve the correct values in all situations. It's completely my fault for not implementing it correctly the first time, I was only thinking about amd64.
This is PR has a number of improvements, I suggest reading the diffs per-commit rather than a whole.
First of all, it drops support for OpenBSD <6.3, which was EOL'd more than three years ago! This allows to semplify the code a bit and to "const-ify" some parameters.
The second commit is aestetic: it just fixes some typos.
The third commit changes the way we parse the data we get from
common.CallSyscall. Should make this library work on big endians.The fourth commit is the real fix: it changes the way we obtain statistics from the CPU. There were multiple things wrong there:
KERN_CPTIMEreturns an array oflongs, whileKERN_CPTIME2an array of uint64_t. longs depends on the platform (on amd64 are 64bits, on i386 are 32bits).KERN_CPUSTATSthat gives us the statistics and an handy flags field from which we can observe is the cpu is online or not.I've tested this on amd64 (4 real cores, works correctly with both
hw.smtenabled and not) and i386 (2 real cores, no smt), both running OpenBSD-CURRENT.