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

Patch cpu count physical #1727

Merged
merged 1 commit into from Oct 17, 2020

Conversation

jandrovins
Copy link
Contributor

Solves #1620

Method 1 of cpu_count_physical() fails on several scenarios. I tested in a node with two sockets:

$ cat /sys/devices/system/cpu/cpu[0-9]*/topology/core_id | sort
0
0
1
1
2
2
3
3
4
4
5
5
6
6
7
7

Each of these values is added to a set, so this function would end up returning half the physical cores. And in my PC with 2 physical cores with Hyper Threadding:

 $ cat /sys/devices/system/cpu/cpu[0-9]*/topology/core_id
0
1
2
3

This function returned twice the physical cores.

@chaneyzorn
Copy link

chaneyzorn commented Apr 20, 2020

I track the commit history of method 1, it's not a perfect solution to delete it simply.

#1404

"/sys/devices/system/cpu/cpu[0-9]*/topology/core_id"
"/sys/devices/system/cpu/cpu[0-9]*/topology/physical_package_id"

Maybe we should combine the two.

@jandrovins
Copy link
Contributor Author

jandrovins commented Apr 24, 2020

That would solve the issue on systems with more than one socket, but not in systems with Hyper Threading, as physical_package_id is the same on all logical cores, but core_id is different.

But you are right in that the first method solved an existing problem. I believe that ls /sys/devices/system/node/node[0-9]*/ list the attributes of each socket, and hence we could find each core_id associated with each socket. This might be the solution, but it's late where I live, I'll probably post another possible solution this weekend. 😄

@jandrovins
Copy link
Contributor Author

With the new changes, cpu_count_physical fetches thread_siblings_lists values for each logical cpu (i.e. /sys/devices/system/cpu/cpu[0-9]*/topology/thread_siblings_list). This file contains the logical core ids with which the logical core being fetch shares the core. This ids refers to N in cpuN from the paths accessed with glob.

Without Hyper Threading:

$ cat /sys/devices/system/cpu/cpu30/topology/thread_siblings_list
30
$ cat /sys/devices/system/cpu/cpu30/topology/core_id              
15

With Hyper Threading:

$ cat /sys/devices/system/cpu/cpu30/topology/thread_siblings_list
30,62
$ cat /sys/devices/system/cpu/cpu30/topology/core_id
15

@marxin
Copy link
Contributor

marxin commented May 8, 2020

I see very similar problem on 2 socket system in which I have a VM:

In [2]: psutil.cpu_count()                                                                                                                                                                                                                                                      
Out[2]: 64

In [3]: psutil.cpu_count(logical=False)                                                                                                                                                                                                                                         
Out[3]: 1

where I see:

$ cat /sys/devices/system/cpu/cpu[0-9]*/topology/core_id | sort | uniq -c
     64 0

So I see 0 64 times.

@marxin
Copy link
Contributor

marxin commented May 8, 2020

And lscpu tells:

lscpu
Architecture:                    x86_64
CPU op-mode(s):                  32-bit, 64-bit
Byte Order:                      Little Endian
Address sizes:                   42 bits physical, 48 bits virtual
CPU(s):                          64
On-line CPU(s) list:             0-63
Thread(s) per core:              1
Core(s) per socket:              1
Socket(s):                       64
NUMA node(s):                    1
Vendor ID:                       GenuineIntel
CPU family:                      6
Model:                           94
Model name:                      Intel Core Processor (Skylake, IBRS)
Stepping:                        3
CPU MHz:                         2800.046
BogoMIPS:                        5600.09
Virtualization:                  VT-x
Hypervisor vendor:               KVM
Virtualization type:             full
L1d cache:                       2 MiB
L1i cache:                       2 MiB
L2 cache:                        256 MiB
L3 cache:                        1 GiB
NUMA node0 CPU(s):               0-63
Vulnerability Itlb multihit:     KVM: Vulnerable
Vulnerability L1tf:              Mitigation; PTE Inversion; VMX vulnerable, SMT disabled
Vulnerability Mds:               Vulnerable; SMT Host state unknown
Vulnerability Meltdown:          Vulnerable
Vulnerability Spec store bypass: Vulnerable
Vulnerability Spectre v1:        Vulnerable: __user pointer sanitization and usercopy barriers only; no swapgs barriers
Vulnerability Spectre v2:        Vulnerable, IBPB: disabled, STIBP: disabled
Vulnerability Tsx async abort:   Vulnerable
Flags:                           fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid tsc_known_freq pni pclmulqdq vmx ssse3 fma cx16 pcid sse4_1 sse4_2
                                  x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch cpuid_fault invpcid_single ssbd ibrs ibpb stibp tpr_shadow vnmi flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2
                                  erms invpcid rtm rdseed adx smap xsaveopt arat umip md_clear arch_capabilities

@ml-evs
Copy link

ml-evs commented Jun 4, 2020

Thanks @jandrovins for this PR!

I've just tested it out on our dual-socket CPU E5-2660 v3 machine on Linux. It looks like the number of cores is now correctly reported, taking hyperthreading into account. Though these CPUs support hyperthreading (flag: ht present in /proc/cpuinfo), I believe we have it disabled (see lscpu output at bottom "Thread per core"). I guess it would be useful to test this on a multi-socket machine that has HT enabled?

$ uname -a
Linux node14 3.11.10-25-default #1 SMP Wed Dec 17 17:57:03 UTC 2014 (8210f77) x86_64 x86_64 x86_64 GNU/Linux
$ cat /proc/cpuinfo | grep processor | tail -n 1
processor       : 19

Old behaviour:

>>> import psutil
>>> psutil.cpu_count()  # correct, by accident? i.e. guessing that HT is turned on, and so doubling the core count for single socket
20
>>> psutil.cpu_count(logical=False)
10
>>> psutil._pslinux.cpu_count_physical()
10

With this PR, everything seems correct:

>>> import psutil
>>> psutil.cpu_count()
20
>>> psutil.cpu_count(logical=False)
20
>>> psutil._pslinux.cpu_count_physical()
20

lscpu output:

$ lscpu
Architecture:          x86_64
CPU op-mode(s):        32-bit, 64-bit
Byte Order:            Little Endian
CPU(s):                20
On-line CPU(s) list:   0-19
Thread(s) per core:    1
Core(s) per socket:    10
Socket(s):             2
NUMA node(s):          2
Vendor ID:             GenuineIntel
CPU family:            6
Model:                 63
Model name:            Intel(R) Xeon(R) CPU E5-2660 v3 @ 2.60GHz
...

@Keou0007
Copy link

Keou0007 commented Jun 5, 2020

I have also tested the new method 1 on dual socket machines both with and without HT. It returns correct values under CentOS 6 and 7.

@dgold9670
Copy link

This looks to be ready to merge. Am I missing a reason it hasn't been?

@ewilts
Copy link

ewilts commented Aug 20, 2020

@jandrovins will this patch also fix issue 1808 that I reported yesterday?
#1808
My issue isn't hyper-threading related but simply a dual-socket 8-cores per socket system returning 15 cores. The symptoms above don't appear to address an "off-by-1" issue.

@ml-evs
Copy link

ml-evs commented Aug 20, 2020

@jandrovins will this patch also fix issue 1808 that I reported yesterday?
#1808
My issue isn't hyper-threading related but simply a dual-socket 8-cores per socket system returning 15 cores. The symptoms above don't appear to address an "off-by-1" issue.

Could you try it @ewilts?

I've just tested on a dual-socket Xeon Gold 6244 (8 cores, 16 threads per socket) machine, and the current psutil release gives 12 (?!) and 32 for psutil.cpu_count(logical={False, True}). This looks similar to the machine you tested on in #1808.

Installing from this patch pip install git+http://github.com/jandrovins/psutil@patch_cpu_count_physical gives the correct 16/32.

Lomanic added a commit to shirou/gopsutil that referenced this pull request Sep 14, 2020
Lomanic added a commit to shirou/gopsutil that referenced this pull request Sep 15, 2020
Lomanic added a commit to shirou/gopsutil that referenced this pull request Sep 15, 2020
shirou added a commit to shirou/gopsutil that referenced this pull request Sep 19, 2020
[cpu][linux] Fix #849 implement giampaolo/psutil#1727 and test Counts against lscpu
@PedanticHacker
Copy link

PedanticHacker commented Sep 24, 2020

If you wanna know whether the processor running your code is utilizing hyperthreading and to then act accordingly, you can define your own function.

import psutil

def hyperthreading_enabled():
    return psutil.cpu_count(logical=True) > psutil.cpu_count(logical=False)

if hyperthreading_enabled():
    # Act on whether hyperthreading is enabled!
else:
    # Act on whether hyperthreading is disabled!

@ml-evs
Copy link

ml-evs commented Sep 24, 2020

If you wanna know whether the processor running your code is utilizing hyperthreading and to then act accordingly, you can define your own function.

import psutil

def hyperthreading_enabled():
    return psutil.cpu_count(logical=True) > psutil.cpu_count(logical=False)

if hyperthreading_enabled():
    # Act on whether hyperthreading is enabled!
else:
    # Act on whether hyperthreading is disabled!

This doesn't work for multi-socket CPUs (as discussed above). If you have hyperthreading disabled and 2 sockets (a common configuration) then you get different answers, even with HT disabled.

@PedanticHacker
Copy link

This doesn't work for multi-socket CPUs (as discussed above). If you have hyperthreading disabled and 2 sockets (a common configuration) then you get different answers, even with HT disabled.

There are not a lot of people in these days who have more than 1 physical CPU on their motherboard. That was once a thing, but now it isn't anymore (since the multicore CPUs came into market). And even if one has a multisocket CPU, one must have Windows 10 Pro (the Home edition supports only single-socket CPUs).

I know that we want to detect every possible scenario, but if one has Windows 10 Home, then you can be sure that only 1 physical CPU is being used (albeit one has 2 or more physical CPUs).

I am googling around for a way to detect whether a multisocket CPU is utilized by the system. Haven't found anything useful yet.

@ewilts
Copy link

ewilts commented Sep 24, 2020

There are not a lot of people in these days who have more than 1 physical CPU on their motherboard

For home computers, you're probably right. For data centers, this is absolutely false.

And even if one has a multisocket CPU, one must have Windows 10 Pro

Or Linux, which is where I tripped over this bug and where most of this thread has been focused.

@ml-evs
Copy link

ml-evs commented Sep 24, 2020

I am googling around for a way to detect whether a multisocket CPU is utilized by the system. Haven't found anything useful yet.

This is precisely one of the issues that this PR solves.

@PedanticHacker
Copy link

When is the new version of psutil gonna be available that fixes this?

@giampaolo
Copy link
Owner

giampaolo commented Sep 24, 2020

@all-involved: I trust this PR fixes the problem which unfortunately I cannot reproduce or investigate due to lack of time. Before merging though, I would like to make sure that also "method 2" is not broken:

psutil/psutil/_pslinux.py

Lines 630 to 651 in ba0c0ab

# Method #2
mapping = {}
current_info = {}
with open_binary('%s/cpuinfo' % get_procfs_path()) as f:
for line in f:
line = line.strip().lower()
if not line:
# new section
if (b'physical id' in current_info and
b'cpu cores' in current_info):
mapping[current_info[b'physical id']] = \
current_info[b'cpu cores']
current_info = {}
else:
# ongoing section
if (line.startswith(b'physical id') or
line.startswith(b'cpu cores')):
key, value = line.split(b'\t:', 1)
current_info[key] = int(value)
result = sum(mapping.values())
return result or None # mimic os.cpu_count()

"method 2" is used if /sys/devices/system/cpu/cpu[0-9]*/topology/thread_siblings_list files are not available. What happens if "method 1" is removed/commented-out? Does "method 2" give the expected results? Can somebody try?

@Keou0007
Copy link

tested on CentOS 6 and 7, both method 1 and 2 independently return the correct number of physical cores across two sockets.

@PedanticHacker
Copy link

Let me get this straight. So does the cpu_count_physical() return the number of physical CPU cores or the physical CPU sockets? The name is a little bit misleading.

Also, the name of the function cpu_count() suggests to me that the function counts how many physical CPUs a system has. I think it'd be better if the name was thread_count() for counting CPU threads, and core_count() for counting CPU cores, and then cpu_count() should count physical CPUs in a system, so CPU sockets.

Do you find any value in this?

@dgold9670
Copy link

dgold9670 commented Sep 25, 2020

Using cat /proc/cpuinfo and lscpu as standins, it looks like physical_id is physical socket, core id is specific core number, and cpu cores the number of physical (non-hyperthreaded) cores per physical socket.
Output:

bash-4.2# lscpu |grep -E 'CPU|Thread|Core|Socket'
CPU op-mode(s): 32-bit, 64-bit
CPU(s): 32
On-line CPU(s) list: 0-31
Thread(s) per core: 2
Core(s) per socket: 8
Socket(s): 2
CPU family: 6
Model name: Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz
CPU MHz: 3011.328
CPU max MHz: 3300.0000
CPU min MHz: 1200.0000
NUMA node0 CPU(s): 0-7,16-23
NUMA node1 CPU(s): 8-15,24-31
bash-4.2# cat /proc/cpuinfo |grep core|sort -u
core id : 0
core id : 1
core id : 2
core id : 3
core id : 4
core id : 5
core id : 6
core id : 7
cpu cores : 8
bash-4.2# cat /proc/cpuinfo |grep physical|sort -u
physical id : 0
physical id : 1

@giampaolo
Copy link
Owner

giampaolo commented Sep 25, 2020

Let me get this straight. So does the cpu_count_physical() return the number of physical CPU cores or the physical CPU sockets? The name is a little bit misleading.

It's supposed to return physical CPU cores. And yes, it's a bit misleading. To summarize:

  • cpu_count(logical=True) = number of logical usable CPUs (including hyper threading ones)
  • cpu_count(logical=False) = number of physical CPU cores

The number of CPU sockets is currently not implemented. We can debate whether to add it but:

  • is there a use-case for it?
  • I'm not sure how to expose that in terms of API; I wish cpu_count(logical=bool) was cpu_count(type="string"). That would have been good also because somebody requested the number of CPU NUMA nodes on Windows, but that boat has sailed long ago.

@amanusk
Copy link
Collaborator

amanusk commented Sep 26, 2020

I agree this functionality is missing in psutil, and would be interested in having it.
I'd like to propose some suggestions for a non breaking API:

  • A new call for socket_count for getting the number of socket. This might help distinguish CPU which usually means core/thread in psutil and an actual separate socket
  • An optional second parameter for cpu_count which receives the index of the socket you wish to get information for. By default, the call will return the summary of both, but e.g. the call cpu_count(logial=false, index=0) would return only the number of physical cores on socket 0 in a multi socket system

@zklaus
Copy link

zklaus commented Oct 13, 2020

Note that thread_siblings_list is deprecated (see here).
The new name is apparently core_cpus_list. I personally also had success with the following code that is based only on all *_id files.

def cpu_count_physical():
    # Adapted from psutil
    """Return the number of physical cores in the system."""
    IDS = ["physical_package_id",
           "die_id",
           "core_id",
           "book_id",
           "drawer_id"]
    # Method #1
    core_ids = set()
    for path in glob.glob(
            "/sys/devices/system/cpu/cpu[0-9]*/topology"):
        core_id = []
        for id in IDS:
            id_path = os.path.join(path, id)
            if os.path.exists(id_path):
                with open(id_path) as f:
                    core_id.append(int(f.read()))
        core_ids.add(tuple(core_id))
    result = len(core_ids)
    if result != 0:
        return result
    else:
        return None

@giampaolo giampaolo merged commit c18ba60 into giampaolo:master Oct 17, 2020
@giampaolo
Copy link
Owner

Merged. I also committed bfae1fc which takes into account that */thread_siblings_list is deprcated and may not be available in the future (*/core_cpus_list is attempted first).
I also added a test which checks that "method 1" and "method 2" return the same value.

Lomanic added a commit to Lomanic/gopsutil that referenced this pull request Nov 6, 2020
giampaolo added a commit that referenced this pull request Dec 21, 2020
This has always been cause of confusion, e.g. see:
#1727 (comment)

Removed the reference to "physical" from dostrings, functions and test.
I still left it in the doc though, as it's more explanatory.

Signed-off-by: Giampaolo Rodola <g.rodola@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet