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

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

> -----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?

OK, that makes sense to me.

Thanks,
Jiayu
> 
> Thanks,
> Maxime
> 
> > Thanks,
> > Jiayu
> >>
> >> What do you think?
> >>
> >> Thanks,
> >> Maxime
> >>
> >>> Jiayu Hu (2):
> >>>   vhost: fix lock on device readiness notification
> >>>   vhost: add thread unsafe async registration functions
> >>>
> >>>  doc/guides/prog_guide/vhost_lib.rst |  12 +++
> >>>  lib/vhost/rte_vhost_async.h         |  42 ++++++++++
> >>>  lib/vhost/version.map               |   4 +
> >>>  lib/vhost/vhost.c                   | 161 +++++++++++++++++++++++++++--------
> -
> >>>  lib/vhost/vhost_user.c              |   5 +-
> >>>  5 files changed, 180 insertions(+), 44 deletions(-)
> >>>
> >