Re: [PATCH 2/3] powerpc: Instrument Hypervisor Calls: add wrappers

Mike Kravetz wrote:

+config HCALL_STATS
+	bool "Hypervisor call instrumentation"
+	depends on PPC_PSERIES
+	help
+	  Adds code to keep track of number of hypervisor calls made and
+	  the amount of time spent in hypervisor calls.
+
+	  This option will add a small amount of overhead to all hypervisor
+	  calls.

Would be good to say how the stats are made available to userspace
here.

+DEFINE_PER_CPU(struct hcall_stats[MAX_HCALL_OPCODES+1], hcall_stats);
+
+static inline void update_stats(unsigned long opcode, unsigned long t_before)
+{
+	unsigned long op_index= opcode >> 2;
+	struct hcall_stats *hs = &__get_cpu_var(hcall_stats[op_index]);
+
+	/* racey, but acceptable for non-critical stats */
+	hs->total_time += (mfspr(SPRN_PURR) - t_before);
+	hs->num_calls++;
+}
+
+extern long plpar_hcall_base (unsigned long opcode,
+                 unsigned long arg1,
+                 unsigned long arg2,
+                 unsigned long arg3,
+                 unsigned long arg4,
+                 unsigned long *out1,
+                 unsigned long *out2,
+                 unsigned long *out3);
+
+long plpar_hcall(unsigned long opcode,
+                 unsigned long arg1,
+                 unsigned long arg2,
+                 unsigned long arg3,
+                 unsigned long arg4,
+                 unsigned long *out1,
+                 unsigned long *out2,
+                 unsigned long *out3)
+{
+	long rc;
+	unsigned long t_before = mfspr(SPRN_PURR);
+
+	rc = plpar_hcall_base(opcode, arg1, arg2, arg3, arg4, out1, out2, out3);
+
+	update_stats(opcode, t_before);
+	return rc;
+}

Without disabling preemption around the mfspr ... update_stats section
in these hcall wrappers, you risk updating the stats on the wrong cpu.

+struct hcall_stats {
+	unsigned long	num_calls;
+	unsigned long	total_time;
+};

A comment explaining the unit of time used for the total_time field
would be nice.