-----Original Message-----
From: Maxime Coquelin <redacted>
Sent: Thursday, July 1, 2021 11:43 PM
To: Hu, Jiayu <redacted>; Maxime Coquelin
[off-list ref]; dev@dpdk.org
Cc: Xia, Chenbo <redacted>; Wang, Yinan
[off-list ref]; David Marchand [off-list ref]
Subject: Re: [PATCH 0/2] provide thread unsafe async registration functions

Hi Jiayu,

On 6/29/21 7:36 AM, Hu, Jiayu wrote:

quoted

Hi Maxime,

quoted

-----Original Message-----
From: Maxime Coquelin <redacted>
Sent: Monday, June 7, 2021 9:20 PM
To: Hu, Jiayu <redacted>; dev@dpdk.org
Cc: maxime.coquelin@redhat.com; Xia, Chenbo <redacted>;
Wang, Yinan [off-list ref]
Subject: Re: [PATCH 0/2] provide thread unsafe async registration
functions

Hi Jiayu,

On 6/7/21 10:07 AM, Hu, Jiayu wrote:

quoted

Hi Maxime,

quoted

-----Original Message-----
From: Maxime Coquelin <redacted>
Sent: Friday, June 4, 2021 3:25 PM
To: Hu, Jiayu <redacted>; dev@dpdk.org
Cc: maxime.coquelin@redhat.com; Xia, Chenbo

[off-list ref];

quoted

quoted

quoted

quoted

Wang, Yinan [off-list ref]
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:

quoted

Lock protection is needed during the vhost notifies the
application of device readiness, so the first patch is to add lock

protection.

quoted

quoted

quoted

quoted

quoted

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?

quoted

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?

quoted

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

quoted

 Thanks,
Jiayu

quoted

Thanks,
Maxime

quoted

Thanks,
Jiayu

quoted

What do you think?

Thanks,
Maxime