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

windows with multiple processor groups, cpu.Percent(percpu=true) returns cpus of any one processor group only. #887

Open
msays2000 opened this issue May 31, 2020 · 5 comments

Comments

@msays2000
Copy link

msays2000 commented May 31, 2020

Describe the bug
In a windows instance having 36 cpu i.e having 36 * 2 logical cpus, windows groups CPU's into batch of 64 logical cpu in a processor group. In this example, it will have

  • 64 logical cpus in processor group A
  • 8 logical cpus in processor group B

gopsutil's cpu.Percent(percpu=true) randomly picks a processor group only and surfaces its result. In above example, if you run once it will log per cpu results showing 64 cpu's. It you stop and run again it will show the other processor group of 8 cpus.

To Reproduce

package main

import (
    "fmt"

    "github.com/shirou/gopsutil/cpu"
)

const (
	CPUPercentDuration = time.Millisecond * 200
)

func main() {
    cpuPercents, err := cpu.Percent(CPUPercentDuration, true)

    fmt.Printf("logical cpus %v \n", cpuPercents)

}

Expected behavior
Expected is it should return total cpu info of 72 logical cores consistently.
Actual is it returns either 64 or 8 cpus.

Environment (please complete the following information):

  • [X ] Windows: [Windows 2016 Server Standard]

Additional context
Noticed the bug is calling an API that is probably not CPU processor group aware.
https://github.com/shirou/gopsutil/blob/master/cpu/cpu_windows.go#L188-L193

        // Invoke windows api proc.
	// The returned err from the windows dll proc will always be non-nil even when successful.
	// See https://godoc.org/golang.org/x/sys/windows#LazyProc.Call for more information
	retCode, _, err := common.ProcNtQuerySystemInformation.Call(
		win32_SystemProcessorPerformanceInformationClass, // System Information Class -> SystemProcessorPerformanceInformation
		uintptr(unsafe.Pointer(&resultBuffer[0])),        // pointer to first element in result buffer
		bufferSize,                        // size of the buffer in memory
		uintptr(unsafe.Pointer(&retSize)), // pointer to the size of the returned results the windows proc will set this
	)

In https://github.com/shirou/gopsutil/blob/master/internal/common/common_windows.go#L64
ProcNtQuerySystemInformation = ModNt.NewProc("NtQuerySystemInformation")

Probably we should make use of NtQuerySystemInformationEx
This function gets a wide range of system properties but allows for refining the query by specifying such things as a processor group.

@msays2000 msays2000 changed the title In windows with multiple processor groups, cpu.Percent(percpu=true) returns cpus info of any one processor group only. windows with multiple processor groups, cpu.Percent(percpu=true) returns cpus of any one processor group only. May 31, 2020
@msays2000
Copy link
Author

CPU_ProcessorGroups

@Lomanic
Copy link
Collaborator

Lomanic commented May 31, 2020

Thanks for your research, I hope to be able to exploit NtQuerySystemInformationEx, the calling convention is different compared to NtQuerySystemInformation, with two more parameters, an input buffer and its length.

We will need to get all processor groups with GetLogicalProcessorInformationEx and call NtQuerySystemInformationEx for each of them (InputBuffer will be the group number a I understand it). In fact I already started investigating this function for #628 to get the number of physical cores without WMI, and it's not straighforward to use in Go.

This document gives some information.

@msays2000
Copy link
Author

msays2000 commented Jun 1, 2020

NtQuerySystemInformationEx is available from version >= 6.1 (windows 7).
Please fall back to use NtQuerySystemInformation <= 6.0 (windows vista and below,)

A reference usage: https://github.com/r3p3r/WindowsInternals/blob/05638d4d28cfb1bdfe7d7d1d1331d5be541f91a3/CpuSet/CpuSet.cpp#L180-L182

https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions

@alvarocabanas
Copy link

Hello, @Lomanic and @msays2000, what is the status of this issue, do you know if there are any plans to fix it?
We have a Bare Metal windows 22 server with 2 sockets of 46 cores and we are facing the issue described here, sometimes gets the Cpu.Percent from one processor group and sometimes from the other, so we get wrong utilization stats.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants