Re: [dpdk-dev] [PATCH v6 2/2] eal: add asynchronous request API to DPDK IPC

[Thomas Monjalon <thomas@monjalon.net>]

28/03/2018 12:42, Burakov, Anatoly:
> On 28-Mar-18 10:53 AM, Thomas Monjalon wrote:
> > 28/03/2018 11:21, Burakov, Anatoly:
> >> so i don't think
> >> it's such a big deal to have two IPC threads instead of one. I'm open to
> >> suggestions on how to make this work without a second thread, but i
> >> don't see it.
> > 
> > I am not against the second thread.
> > I am against both threads :)
> 
> Well, the first thread is already in DPDK. To provide some context, 
> first implementation for DPDK IPC was suggested for 17.11, and (without 
> many conceptual changes) was merged in 18.02. I think it's a bit late to 
> be against both threads :)

No, it's never too late to discuss.
Merging a patch does not prevent discussing it :)

> >> We've discussed possibility of using rte_service internally, but decided
> >> against it for reasons already outlined by Harry - it's not a suitable
> >> mechanism for this kind of thing, not as it is.
> >>
> >> Using interrupt thread for this _will_ work, however this will require a
> >> a lot more changes, as currently alarm API allocates everything through
> >> rte_malloc, while we want to use IPC for rte_malloc (which would make it
> >> a circular dependency). So it'll probably be more API and more
> >> complexity for dealing with malloc vs rte_malloc allocations. Hence the
> >> least-bad approach taken here: a new thread.
> > 
> > If everybody is happy enough with "least bad" design and not trying
> > to improve the core design, what can I say?
> 
> I'm not against trying to improve the core design. I'm just saying that, 
> had this kind of feedback been provided just a bit earlier, I would've 
> had time to fix it in time for deadlines. However, because memory rework 
> patchset depends on this API, i would suggest merging it in now, as is, 
> and commit to a roadmap of improvements for next release(s).

Actually, you had the feedback yourself from the beginning.
You decided to gave up with interrupt thread because its implementation
is not complete (and maybe far from perfect).
There are some communities where it is not acceptable to workaround
core issues because of timing issues. I think we accept it in DPDK,
but I continue to question it, in order to be sure that everybody is OK
with this kind of tradeoff.

> For starters, we could plan on removing alarm thread's dependency on 
> rte_malloc and just use regular malloc API's in there, and rework 
> asynchronous IPC API to use that instead. This shouldn't be much work, 
> and will presumably make you halfway happy, as one of the threads will 
> be gone :)
> 
> We can then look into removing the second thread and moving the entirety 
> of DPDK IPC into the interrupt thread. I'm not too sure how would that 
> work, but i haven't looked at it in any detail, so maybe it is feasible.
> 
> Can we agree on this? It would be great to do everything perfectly from 
> the first try, but having a goal in sight and working towards it is fine 
> too, even if not all of the steps we take are perfect.

The main concern is API.
If all these changes are internal only, and does not involve any major
API change, then I guess it is OK to pospone them in next release.