Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <mcoqu...@redhat.com> > Sent: Monday, June 7, 2021 9:20 PM > To: Hu, Jiayu <jiayu...@intel.com>; dev@dpdk.org > Cc: maxime.coque...@redhat.com; Xia, Chenbo <chenbo....@intel.com>; > Wang, Yinan <yinan.w...@intel.com> > Subject: Re: [PATCH 0/2] provide thread unsafe async registration functions > > Hi Jiayu, > > On 6/7/21 10:07 AM, Hu, Jiayu wrote: > > Hi Maxime, > > > >> -----Original Message----- > >> From: Maxime Coquelin <mcoqu...@redhat.com> > >> Sent: Friday, June 4, 2021 3:25 PM > >> To: Hu, Jiayu <jiayu...@intel.com>; dev@dpdk.org > >> Cc: maxime.coque...@redhat.com; Xia, Chenbo <chenbo....@intel.com>; > >> Wang, Yinan <yinan.w...@intel.com> > >> Subject: Re: [PATCH 0/2] provide thread unsafe async registration > >> functions > >> > >> Sorry, for previous blank reply. > >> > >> On 5/28/21 10:11 AM, Jiayu Hu wrote: > >>> Lock protection is needed during the vhost notifies the application > >>> of device readiness, so the first patch is to add lock protection. > >>> After performing locking, existed async vhost registration functions > >>> will cause deadlock, as they acquire lock too. So the second patch > >>> is to provide unsafe registration functions to support calling > >>> within vhost callback functions. > >> > >> I agree the callback should be always protected, and in that case > >> having a new thread-unsafe API makes sense for async registration. > >> > >> Regarding backport, I'm not sure what we should do. > >> > >> Backporting new API is a no-go, but with only backporting patch 1 > >> async feature will be always broken on 20.11 LTS, right? > > > > Yes, if only backporting this fix patch to 20.11 LTS, it may break > > apps who call async registration functions inside vhost callbacks. > > > > How about making this patch not a fix, but a new feature? > > Async will be still broken in v20.11 in this case. > Maybe the better thing would be to remove async support in v20.11, as its > support was quite limited in that release anyway. Does that make sense?
The code of supporting async vhost are beyond 1000 lines. I am afraid that removing such more code in 20.11 LTS may get objected. Can we note async register/unregister only work in new_/destroy_device, and using them in other vhost callback functions will cause deadlock in 20.11 LTS instead? Does it make sense to you? Thanks, Jiayu > > Thanks, > Maxime > > > Thanks, > > Jiayu > >> > >> What do you think? > >> > >> Thanks, > >> Maxime
