On 04/06/2016 10:05 PM, Huang Rui wrote:
On Wed, Apr 06, 2016 at 08:30:25AM -0700, Guenter Roeck wrote:
On Wed, Apr 06, 2016 at 03:44:11PM +0800, Huang Rui wrote:

+static void do_read_registers_on_cu(void *_data)
+{
+       struct fam15h_power_data *data = _data;
+       int cpu, cu;
+
+       cpu = smp_processor_id();
+

Is this function now defined in non-SMP code ? If so, can you point me to the
patch or branch introducing it ? It doesn't seem to be in mainline or in -next
unless I am missing it.


In include/linux/smp.h


#else /* !SMP */

static inline void smp_send_stop(void) { }

/*
  *      These macros fold the SMP functionality into a single CPU system
  */
#define raw_smp_processor_id()                  0

...

/*
  * smp_processor_id(): get the current CPU ID.
  *
  * if DEBUG_PREEMPT is enabled then we check whether it is
  * used in a preemption-safe way. (smp_processor_id() is safe
  * if it's used in a preemption-off critical section, or in
  * a thread that is bound to the current CPU.)
  *
  * NOTE: raw_smp_processor_id() is for internal use only
  * (smp_processor_id() is the preferred variant), but in rare
  * instances it might also be used to turn off false positives
  * (i.e. smp_processor_id() use that the debugging code reports but
  * which use for some reason is legal). Don't use this to hack around
  * the warning message, as your code might not work under PREEMPT.
  */
#ifdef CONFIG_DEBUG_PREEMPT
   extern unsigned int debug_smp_processor_id(void);
# define smp_processor_id() debug_smp_processor_id()
#else
# define smp_processor_id() raw_smp_processor_id()
#endif


Actually smp_processor_id() should returns 0 if we disable CONFIG_SMP.

+       /*
+        * With the new x86 topology modelling, cpu core id actually
+        * is compute unit id.
+        */
+       cu = cpu_data(cpu).cpu_core_id;
+
+       rdmsrl_safe(MSR_F15H_CU_PWR_ACCUMULATOR, &data->cu_acc_power[cu]);
+}
+
+/*
+ * This function is only able to be called when CPUID
+ * Fn8000_0007:EDX[12] is set.
+ */
+static int read_registers(struct fam15h_power_data *data)
+{
+       int this_cpu, ret, cpu;
+       int core, this_core;
+       cpumask_var_t mask;
+
+       ret = zalloc_cpumask_var(&mask, GFP_KERNEL);
+       if (!ret)
+               return -ENOMEM;
+
+       get_online_cpus();
+       this_cpu = smp_processor_id();
+
+       /*
+        * Choose the first online core of each compute unit, and then
+        * read their MSR value of power and ptsc in a single IPI,
+        * because the MSR value of CPU core represent the compute
+        * unit's.
+        */
+       core = -1;
+
+       for_each_online_cpu(cpu) {
+               this_core = topology_core_id(cpu);
+
+               if (this_core == core)
+                       continue;
+
+               core = this_core;
+
Sorry if I missed some context - is it guaranteed that all cores in the same
compute unit are returned next to each other from for_each_online_cpu() ?


Yes, there is a documentation which introduced from v4.6-rc2:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f7be8610bca88e59dd2fd5d98fcbc5031ef0e079

    - topology_core_id();

     The ID of the core to which a thread belongs. It is also printed in 
/proc/cpuinfo
     "core_id."

...

       AMD nomenclature for CMT systems:

         [node 0] -> [Compute Unit 0] -> [Compute Unit Core 0] -> Linux CPU 0
                                      -> [Compute Unit Core 1] -> Linux CPU 1
                  -> [Compute Unit 1] -> [Compute Unit Core 0] -> Linux CPU 2
                                      -> [Compute Unit Core 1] -> Linux CPU 3

ray@hr-ub:~/tip$ cat /proc/cpuinfo | grep "core id"
core id         : 0
core id         : 0
core id         : 1
core id         : 1

"this_core" here actually means the [Compute Unit] id which current
[Compute Unit Core] belongs to. And "cpu" here means the [Compute Unit Core].

Ok, thanks.

Guenter


Reply via email to