> -----Original Message----- > From: Maxime Coquelin <maxime.coque...@redhat.com> > Sent: Thursday, July 1, 2021 11:43 PM > To: Hu, Jiayu <jiayu...@intel.com>; Maxime Coquelin > <mcoqu...@redhat.com>; dev@dpdk.org > Cc: Xia, Chenbo <chenbo....@intel.com>; Wang, Yinan > <yinan.w...@intel.com>; David Marchand <david.march...@redhat.com> > Subject: Re: [PATCH 0/2] provide thread unsafe async registration functions > > Hi Jiayu, > > On 6/29/21 7:36 AM, Hu, Jiayu wrote: > > 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? > > You are right, that removing 1L LoC in LTS might not be the best idea, since > it > will cause conflicts when doing backports later on. > > Maybe the best way is to return -1 at async registration time, with logging an > error?
OK. Thanks, Jiayu > > Thanks, > Maxime > > > Thanks, > > Jiayu > >> > >> Thanks, > >> Maxime > >> > >>> Thanks, > >>> Jiayu > >>>> > >>>> What do you think? > >>>> > >>>> Thanks, > >>>> Maxime > >
