On 10/08/2010 12:22 PM, Rudolf Mayerhofer wrote:
+static SYSTEM_LOGICAL_PROCESSOR_INFORMATION cached_lpi[1024];
You think there are systems with that many CPUs running Wine?
+ for (i = 0; i < (sizeof(cached_lpi) /
sizeof(SYSTEM_LOGICAL_PROCESSOR_INFORMATION)); i++)
To make it easier to understand what you doing here please calculate number
of elements in cached_lpi array with:
sizeof(cached_lpi) / sizeof(cached_lpi[0]);
This way people don't need to look up what type cached_lpi is.
@@ -1069,6 +1193,160 @@ void fill_cpu_info(void)
}
}
fclose(f);
+
+ /* Parse Logical Processor Information in SysFS */
+ {
+ int coreflags = (cached_sci.FeatureSet & CPU_FEATURE_HTT) ? 1 : 0;
+ int cpucount = 0, cachecount, nodecount, i;
+ ULONG_PTR fullprocessormask = 0, processormask;
+
Please don't do this. Don't create a block just so you can declare
variables. Put variable declaration at the top of the outer block.
+ while ((processormask = sysfs_cpu_topology_getmap(cpucount,
"thread_siblings")) > 0)
+ {
+ /* Core information */
+ {
+ /* Unique cores are found multiple times during
iteration, add only once */
+ for (i = 0; i < (sizeof(cached_lpi) /
sizeof(SYSTEM_LOGICAL_PROCESSOR_INFORMATION)); i++)
+ {
...
+ }
+ }
...
+ }
Same thing here.
+static int sysfs_cpucache_getint(int cpu, int cache, const char *type)
+ /* Cache information */
+ cachecount = 0;
+ while ((processormask = sysfs_cpucache_cpumap(cpucount,
cachecount)) > 0)
+ {
+ /* Empty Processormasks are invalid, ignore them */
What does this comment refers too? If it's for the above while() condition,
then please put comment above it, not inside the block.
+ BYTE level = sysfs_cpucache_getint(cpucount, cachecount,
"level");
+ BYTE associativity = sysfs_cpucache_getint(cpucount, cachecount,
"ways_of_associativity");
+ WORD linesize = sysfs_cpucache_getint(cpucount, cachecount,
"coherency_line_size");
+ DWORD size = sysfs_cpucache_getint(cpucount, cachecount,
"size");
I've already told you before - do not mix signed and unsigned. If result of
sysfs_cpucache_getint is always assigned to an unsigned variable then change
it's return type to unsigned type too.
+ /* Determine Cachetype */
+ if (typestring[0] == 'D')
+ {
+ type = CacheData;
+ }
+ else if (typestring[0] == 'I')
+ {
+ type = CacheInstruction;
+ }
+ else if (typestring[0] == 'U')
+ {
+ type = CacheUnified;
+ }
+ else if (typestring[0] == 'T')
+ {
+ type = CacheTrace;
+ }
Either use swith case construct here or use complete words and stricmp.
+ if ((processormask = sysfs_numanode_cpumap(nodecount)) > 0)
+ {
+ /* Numa Nodes are already unique. just add them */
+ do
+ {
+ }
+ while ((processormask = sysfs_numanode_cpumap(nodecount)) > 0);
+ }
+ else
+ {
+ }
People already asked you not to do this. From looking at the code it appears
that there is always one node. So get rid of everything except do{} while()
block. It will do exactly what you want with extra if()s.
Vitaliy.
