Re: [dpdk-dev] [PATCH 0/2] provide thread unsafe async registration functions

["Hu, Jiayu" <jiayu.hu@intel.com>]

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <mcoqueli@redhat.com>
> Sent: Monday, June 7, 2021 9:20 PM
> To: Hu, Jiayu <jiayu.hu@intel.com>; dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> Wang, Yinan <yinan.wang@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 <mcoqueli@redhat.com>
> >> Sent: Friday, June 4, 2021 3:25 PM
> >> To: Hu, Jiayu <jiayu.hu@intel.com>; dev@dpdk.org
> >> Cc: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> >> Wang, Yinan <yinan.wang@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