From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vitaly Kuznetsov Subject: Re: [PATCH v3] hv_netvsc: Add per-cpu ethtool stats for netvsc Date: Tue, 24 Jul 2018 13:00:59 +0200 Message-ID: <87k1pkrj84.fsf@vitty.brq.redhat.com> References: <20180613193608.444-1-yidren@linuxonhyperv.com> <20180724012622.26873-1-yidren@linuxonhyperv.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Stephen Hemminger , "netdev@vger.kernel.org" , Haiyang Zhang , "linux-kernel@vger.kernel.org" , Madhan Sivakumar , "devel@linuxdriverproject.org" , "David S. Miller" To: Yidong Ren Return-path: In-Reply-To: (Yidong Ren's message of "Tue, 24 Jul 2018 01:42:06 +0000") List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" List-Id: netdev.vger.kernel.org Yidong Ren writes: >> From: Yidong Ren >> 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