shirou / gopsutil

psutil for golang
Other
10.56k stars 1.58k forks source link

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

Open msays2000 opened 4 years ago

msays2000 commented 4 years ago

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

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):

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 commented 4 years ago
CPU_ProcessorGroups
msays2000 commented 4 years ago

Some hints in https://www.geoffchappell.com/studies/windows/km/ntoskrnl/api/ex/sysinfo/query.htm https://www.geoffchappell.com/studies/windows/km/ntoskrnl/api/ex/sysinfo/queryex.htm (Ex api)

Lomanic commented 4 years ago

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 commented 4 years ago

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 commented 4 months ago

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.