From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH] kni: add support for core_id param in single threaded mode Date: Wed, 21 Sep 2016 19:23:47 +0100 Message-ID: References: <20160910135016.6468-2-vladyslav.buslov@harmonicinc.com> <20160920181637.26778-1-vladyslav.buslov@harmonicinc.com> <20160920113636.38b2ed2a@xeon-e3> <366f0b2a-abb6-9fdc-cd12-d79185d32c5e@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: "dev@dpdk.org" To: Vladyslav Buslov , Stephen Hemminger Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id C39672952 for ; Wed, 21 Sep 2016 20:23:49 +0200 (CEST) In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 9/21/2016 6:15 PM, Vladyslav Buslov wrote: >> On 9/20/2016 7:36 PM, Stephen Hemminger wrote: >>> On Tue, 20 Sep 2016 21:16:37 +0300 >>> Vladyslav Buslov wrote: >>> >>>> @@ -123,6 +125,9 @@ static int __net_init kni_init_net(struct net *net) >>>> /* Clear the bit of device in use */ >>>> clear_bit(KNI_DEV_IN_USE_BIT_NUM, &knet->device_in_use); >>>> >>>> + mutex_init(&knet->kni_kthread_lock); >>>> + knet->kni_kthread = NULL; >>>> + >>> >>> Why not just use kzalloc() here? You would still need to init the >>> mutex etc, but it would be safer. >>> >> >> Hi Vladyslav, >> >> This is good suggestion, if you send a new version for this update, please >> keep my Ack. >> >> Thanks, >> ferruh > > Hi Ferruh, Stephen, > > Could you please elaborate on using kzalloc for this code. > Currently kni_thread_lock is value member of kni_net structure and never explicitly allocated or deallocated. > Kni_kthread is pointer member of kni_net and is implicitly created and destroyed by kthread_run, kthread_stop functions. > Which one of those do you suggest to allocate with kzalloc() and how would it improve safety? > Currently: kni_init_net() { knet = kmalloc(..); .. mutex_init(..); knet->kni_thread = NULL; } If you allocate knet via kzalloc(), no need to assign NULL to kni_thread. Also this is safer because any uninitialized knet field will be zero instead of random value. This is what I understood at least J Thanks, ferruh