Re: [PATCH v3] hv_netvsc: Add per-cpu ethtool stats for netvsc

Yidong Ren [off-list ref] writes:

quoted

From: Yidong Ren <redacted>
Sent: Monday, July 23, 2018 6:26 PM
+	pcpu_sum = kvmalloc(sizeof(struct netvsc_ethtool_pcpu_stats) *
+			num_present_cpus(), GFP_KERNEL);

Since there is no plan for CPU hotplug in Hyper-V in short term, it is fine 
to use num_present_cpus for now. We can move to debugfs later if necessary.

While you do for_each_present_cpu() in netvsc_get_ethtool_stats(),
netvsc_get_pcpu_stats() does for_each_possible_cpu(). This looks
inconsistent.

The allocation you're doing here is short-lived so I would suggest you
use possible_cpus everywhere. Even knowing there's no CPU hotplug on
Hyper-V at this moment, it can appear later and we'll get a hard-to-find
issue. Moreover, we may consider using netvsc driver on e.g. KVM with
Hyper-V enlightenments and KVM has CPU hotplug already.

-- 
  Vitaly

From: Vitaly Kuznetsov <vkuznets@redhat.com>
While you do for_each_present_cpu() in netvsc_get_ethtool_stats(),
netvsc_get_pcpu_stats() does for_each_possible_cpu(). This looks
inconsistent.

I made a mistake there.  Thanks for catch me.
 

The allocation you're doing here is short-lived so I would suggest you use
possible_cpus everywhere. Even knowing there's no CPU hotplug on Hyper-
V at this moment, it can appear later and we'll get a hard-to-find issue.
Moreover, we may consider using netvsc driver on e.g. KVM with Hyper-V
enlightenments and KVM has CPU hotplug already.

That would cause the output to be very long and useless.
I will submit a revision that allocates for num_possible_cpus, but only copy out stats for each present cpu.

-Yidong