RE: [PATCH v5] net/ice: fix ice dcf control thread crash

["Ye, MingjinX" <mingjinx.ye@intel.com>]

Hi Qi, here is my new solution, can you give me some good suggestions.
1. remove the 'vc_event_msg_cb == NULL' related processing and let each 'ice-rest' thread, end normally.
2. Define vsi_update_thread_num as rte_atomic32_t.

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: 2023年3月20日 20:53
> To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Zhang, Ke1X <ke1x.zhang@intel.com>
> Subject: RE: [PATCH v5] net/ice: fix ice dcf control thread crash

> >  	for (;;) {
> > +		if (hw->vc_event_msg_cb == NULL)
> > +			break;
> Can you explain why this is required, seems it not related with your commit
> log
The purpose of this is to bring all 'ice-reset' threads to a quick end when hw is released.

> >
> > -	rte_intr_enable(pci_dev->intr_handle);
> > -	ice_dcf_enable_irq0(hw);
> > +	if (hw->vc_event_msg_cb != NULL) {
> > +		rte_intr_enable(pci_dev->intr_handle);
> > +		ice_dcf_enable_irq0(hw);
> 
> Same question as above
These are called when HW releases the resource. Therefore, there is no need to call.

> > +	rte_spinlock_lock(&dcf_hw->vsi_thread_lock);
> > +	dcf_hw->vsi_update_thread_num++;
> > +	rte_spinlock_unlock(&dcf_hw->vsi_thread_lock);
> 
> I think you can define vsi_update_thread_num as rte_atomic32_t and use
> rte_atomic32_add/sub
At first I chose the rte_atomic32_t option, which is not very elegant
using spinlock.
The 'checkpatches.sh' script gives a warning ('Using rte_atomicNN_xxx')
when it is executed. I saw the comment on line 89 of the
script (# refrain from new additions of 16/32/64 bits rte_atomicNN_xxx()),
so I went with the spinlock solution.