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.zh...@intel.com>
> Sent: 2023年3月20日 20:53
> To: Ye, MingjinX <mingjinx...@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.y...@intel.com>; sta...@dpdk.org; Zhou, YidingX
> <yidingx.z...@intel.com>; Zhang, Ke1X <ke1x.zh...@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.
