> -----Original Message----- > From: Maxime Coquelin <[email protected]> > Sent: Thursday, July 1, 2021 11:43 PM > To: Hu, Jiayu <[email protected]>; Maxime Coquelin > <[email protected]>; [email protected] > Cc: Xia, Chenbo <[email protected]>; Wang, Yinan > <[email protected]>; David Marchand <[email protected]> > 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 <[email protected]> > >> Sent: Monday, June 7, 2021 9:20 PM > >> To: Hu, Jiayu <[email protected]>; [email protected] > >> Cc: [email protected]; Xia, Chenbo <[email protected]>; > >> Wang, Yinan <[email protected]> > >> 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 <[email protected]> > >>>> Sent: Friday, June 4, 2021 3:25 PM > >>>> To: Hu, Jiayu <[email protected]>; [email protected] > >>>> Cc: [email protected]; Xia, Chenbo > <[email protected]>; > >>>> Wang, Yinan <[email protected]> > >>>> 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 > >
