[PATCH v2 0/5] ntdll: Improve logical core reporting on Linux and Mac.
Hi, This is an updated version of the previous patchset with a few improvements to unify some of the logic better across Linux and Mac. The overall issue is that on Linux we report each logical core as a physical core, so on SMT systems we report twice the number of cores. In addition we don't properly report SMT capabilities. The main issues are in the Linux sysfs code for parsing the CPU topology. We use a common helper function (logical_proc_info_add_by_id) among Linux and Mac. This is were some of the fixes need to be implemented for not adding cores twice. (We do something similar in logical_proc_info_add_cache in relation to parsing shared_cpu_map.) However we use logical_proc_info_add_by_id in an inconsistent manner between Linux and Mac code. On Linux we use different ID values (core and package), but on Mac we re-use the package ID. The early patches rectify this and thanks Andrew Eikum for helping to test this. The later patches fix the Linux issues and then add proper SMT reporting flags. Thanks, Roderick Roderick Colenbrander (5): ntdll: Derive number of logical CPU cores from core mask. ntdll: store core information by core on apple. ntdll: Report physical cores once with proper thread mask. ntdll: GetLogicalProcessorInformationEx report LTP_PC_SMT for SMT cores. ntdll: GetLogicalProcessorInformation report LPT_PC_SMT for SMT cores. dlls/ntdll/nt.c | 75 +++++++++++++++++++++++++++++++++++++++++++++------------ include/winnt.h | 2 ++ 2 files changed, 61 insertions(+), 16 deletions(-) -- 2.14.4
Signed-off-by: Roderick Colenbrander <thunderbird2k(a)gmail.com> --- dlls/ntdll/nt.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index dc0ce04f42..e6458b98f2 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -1511,6 +1511,17 @@ static inline BOOL logical_proc_info_add_group(SYSTEM_LOGICAL_PROCESSOR_INFORMAT } #ifdef linux +static DWORD count_bits(ULONG_PTR mask) +{ + DWORD count = 0; + while (mask > 0) + { + mask >>= 1; + count++; + } + return count; +} + /* for 'data', max_len is the array count. for 'dataex', max_len is in bytes */ static NTSTATUS create_logical_proc_info(SYSTEM_LOGICAL_PROCESSOR_INFORMATION **data, SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX **dataex, DWORD *max_len) @@ -1643,7 +1654,6 @@ static NTSTATUS create_logical_proc_info(SYSTEM_LOGICAL_PROCESSOR_INFORMATION ** for(i=0; i<len; i++){ if((*data)[i].Relationship == RelationProcessorCore){ all_cpus_mask |= (*data)[i].ProcessorMask; - ++num_cpus; } } }else{ @@ -1651,11 +1661,11 @@ static NTSTATUS create_logical_proc_info(SYSTEM_LOGICAL_PROCESSOR_INFORMATION ** SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *infoex = (SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *)(((char *)*dataex) + i); if(infoex->Relationship == RelationProcessorCore){ all_cpus_mask |= infoex->u.Processor.GroupMask[0].Mask; - ++num_cpus; } i += infoex->Size; } } + num_cpus = count_bits(all_cpus_mask); fnuma_list = fopen("/sys/devices/system/node/online", "r"); if(!fnuma_list) -- 2.14.4
The id parameter can be used for core / package lookup. On Linux we pass package or core values. On Mac, we use package for both. Use a core identifier on Mac as well. Signed-off-by: Roderick Colenbrander <thunderbird2k(a)gmail.com> --- dlls/ntdll/nt.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index e6458b98f2..14e18dc6af 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -1309,6 +1309,12 @@ static DWORD log_proc_ex_size_plus(DWORD size) return sizeof(LOGICAL_PROCESSOR_RELATIONSHIP) + sizeof(DWORD) + size; } +/* Store package and core information for a logical processor. Parsing of processor + * data may happen in multiple passes; the 'id' parameter is then used to locate + * previously stored data. The type of data stored in 'id' depends on 'rel': + * - RelationProcessorPackage: package id ('CPU socket'). + * - RelationProcessorCore: physical core number. + */ static inline BOOL logical_proc_info_add_by_id(SYSTEM_LOGICAL_PROCESSOR_INFORMATION **pdata, SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX **pdataex, DWORD *len, DWORD *pmax_len, LOGICAL_PROCESSOR_RELATIONSHIP rel, DWORD id, ULONG_PTR mask) @@ -1805,6 +1811,7 @@ static NTSTATUS create_logical_proc_info(SYSTEM_LOGICAL_PROCESSOR_INFORMATION ** for(p = 0; p < pkgs_no; ++p){ for(j = 0; j < cores_per_package && p * cores_per_package + j < cores_no; ++j){ ULONG_PTR mask = 0; + DWORD phys_core; for(k = 0; k < lcpu_per_core; ++k) mask |= (ULONG_PTR)1 << (j * lcpu_per_core + k); @@ -1816,7 +1823,8 @@ static NTSTATUS create_logical_proc_info(SYSTEM_LOGICAL_PROCESSOR_INFORMATION ** return STATUS_NO_MEMORY; /* add new core */ - if(!logical_proc_info_add_by_id(data, dataex, &len, max_len, RelationProcessorCore, p, mask)) + phys_core = p * cores_per_package + j; + if(!logical_proc_info_add_by_id(data, dataex, &len, max_len, RelationProcessorCore, phys_core, mask)) return STATUS_NO_MEMORY; for(i = 1; i < 5; ++i){ -- 2.14.4
Signed-off-by: Roderick Colenbrander <thunderbird2k(a)gmail.com> --- dlls/ntdll/nt.c | 48 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index 14e18dc6af..b16f320957 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -1322,17 +1322,16 @@ static inline BOOL logical_proc_info_add_by_id(SYSTEM_LOGICAL_PROCESSOR_INFORMAT if (pdata) { DWORD i; - if(rel == RelationProcessorPackage){ - for(i=0; i<*len; i++) + for (i=0; i<*len; i++) + { + if (rel == RelationProcessorPackage && (*pdata)[i].Relationship == rel && (*pdata)[i].u.Reserved[1] == id) { - if ((*pdata)[i].Relationship!=rel || (*pdata)[i].u.Reserved[1]!=id) - continue; - (*pdata)[i].ProcessorMask |= mask; return TRUE; } - }else - i = *len; + else if (rel == RelationProcessorCore && (*pdata)[i].Relationship == rel && (*pdata)[i].u.Reserved[1] == id) + return TRUE; + } while(*len == *pmax_len) { @@ -1358,6 +1357,10 @@ static inline BOOL logical_proc_info_add_by_id(SYSTEM_LOGICAL_PROCESSOR_INFORMAT dataex->u.Processor.GroupMask[0].Mask |= mask; return TRUE; } + else if (rel == RelationProcessorCore && dataex->Relationship == rel && dataex->u.Processor.Reserved[1] == id) + { + return TRUE; + } ofs += dataex->Size; } @@ -1554,6 +1557,9 @@ static NTSTATUS create_logical_proc_info(SYSTEM_LOGICAL_PROCESSOR_INFORMATION ** for(i=beg; i<=end; i++) { + DWORD phys_core = 0; + ULONG_PTR thread_mask = 0; + if(i > 8*sizeof(ULONG_PTR)) { FIXME("skipping logical processor %d\n", i); @@ -1574,15 +1580,35 @@ static NTSTATUS create_logical_proc_info(SYSTEM_LOGICAL_PROCESSOR_INFORMATION ** return STATUS_NO_MEMORY; } - sprintf(name, core_info, i, "core_id"); + /* Sysfs enumerates logical cores (and not physical cores), but Windows enumerates + * by physical core. Upon enumerating a logical core in sysfs, we register a physical + * core and all its logical cores. In order to not report physical cores multiple + * times, we pass a unique physical core ID to logical_proc_info_add_by_id and let + * that call figure out any duplication. + * Obtain a unique physical core ID from the first element of thread_siblings_list. + * This list provides logical cores sharing the same physical core. The IDs are based + * on kernel cpu core numbering as opposed to a hardware core ID like provided through + * 'core_id', so are suitable as a unique ID. + */ + sprintf(name, core_info, i, "thread_siblings_list"); f = fopen(name, "r"); if(f) { - fscanf(f, "%u", &r); + fscanf(f, "%d%c", &phys_core, &op); + fclose(f); + } + else phys_core = i; + + /* Mask of logical threads sharing same physical core in kernel core numbering. */ + sprintf(name, core_info, i, "thread_siblings"); + f = fopen(name, "r"); + if(f) + { + fscanf(f, "%lx", &thread_mask); fclose(f); } - else r = i; - if(!logical_proc_info_add_by_id(data, dataex, &len, max_len, RelationProcessorCore, r, (ULONG_PTR)1 << i)) + else thread_mask = 1<<i; + if(!logical_proc_info_add_by_id(data, dataex, &len, max_len, RelationProcessorCore, phys_core, thread_mask)) { fclose(fcpu_list); return STATUS_NO_MEMORY; -- 2.14.4
Signed-off-by: Roderick Colenbrander <thunderbird2k(a)gmail.com> --- dlls/ntdll/nt.c | 27 +++++++++++++++------------ include/winnt.h | 2 ++ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index b16f320957..5712e4cc1b 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -1309,6 +1309,17 @@ static DWORD log_proc_ex_size_plus(DWORD size) return sizeof(LOGICAL_PROCESSOR_RELATIONSHIP) + sizeof(DWORD) + size; } +static DWORD count_bits(ULONG_PTR mask) +{ + DWORD count = 0; + while (mask > 0) + { + mask >>= 1; + count++; + } + return count; +} + /* Store package and core information for a logical processor. Parsing of processor * data may happen in multiple passes; the 'id' parameter is then used to locate * previously stored data. The type of data stored in 'id' depends on 'rel': @@ -1377,7 +1388,10 @@ static inline BOOL logical_proc_info_add_by_id(SYSTEM_LOGICAL_PROCESSOR_INFORMAT dataex->Relationship = rel; dataex->Size = log_proc_ex_size_plus(sizeof(PROCESSOR_RELATIONSHIP)); - dataex->u.Processor.Flags = 0; /* TODO */ + if (rel == RelationProcessorCore) + dataex->u.Processor.Flags = count_bits(mask) > 1 ? LTP_PC_SMT : 0; + else + dataex->u.Processor.Flags = 0; dataex->u.Processor.EfficiencyClass = 0; dataex->u.Processor.GroupCount = 1; dataex->u.Processor.GroupMask[0].Mask = mask; @@ -1520,17 +1534,6 @@ static inline BOOL logical_proc_info_add_group(SYSTEM_LOGICAL_PROCESSOR_INFORMAT } #ifdef linux -static DWORD count_bits(ULONG_PTR mask) -{ - DWORD count = 0; - while (mask > 0) - { - mask >>= 1; - count++; - } - return count; -} - /* for 'data', max_len is the array count. for 'dataex', max_len is in bytes */ static NTSTATUS create_logical_proc_info(SYSTEM_LOGICAL_PROCESSOR_INFORMATION **data, SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX **dataex, DWORD *max_len) diff --git a/include/winnt.h b/include/winnt.h index 54bf11dabd..862cc45291 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -5982,6 +5982,8 @@ typedef enum _LOGICAL_PROCESSOR_RELATIONSHIP RelationAll = 0xffff } LOGICAL_PROCESSOR_RELATIONSHIP; +#define LTP_PC_SMT 0x1 + typedef enum _PROCESSOR_CACHE_TYPE { CacheUnified, -- 2.14.4
Signed-off-by: Roderick Colenbrander <thunderbird2k(a)gmail.com> --- dlls/ntdll/nt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index 5712e4cc1b..2ca9cd59b0 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -1352,7 +1352,8 @@ static inline BOOL logical_proc_info_add_by_id(SYSTEM_LOGICAL_PROCESSOR_INFORMAT (*pdata)[i].Relationship = rel; (*pdata)[i].ProcessorMask = mask; - /* TODO: set processor core flags */ + if (rel == RelationProcessorCore) + (*pdata)[i].u.ProcessorCore.Flags = count_bits(mask) > 1 ? LTP_PC_SMT : 0; (*pdata)[i].u.Reserved[0] = 0; (*pdata)[i].u.Reserved[1] = id; *len = i+1; -- 2.14.4
participants (1)
-
Roderick Colenbrander