Re: [dpdk-dev] [PATCH] kni: add support for core_id param in single threaded mode

[Ferruh Yigit <ferruh.yigit@intel.com>]

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 <vladyslav.buslov@harmonicinc.com> 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