30/10/2020 17:02, McDaniel, Timothy: > From: Thomas Monjalon <tho...@monjalon.net> > > 30/10/2020 16:35, McDaniel, Timothy: > > > From: Thomas Monjalon <tho...@monjalon.net> > > > > 30/10/2020 12:58, McDaniel, Timothy: > > > > > From: McDaniel, Timothy > > > > > > From: Thomas Monjalon <tho...@monjalon.net> > > > > > > > 30/10/2020 10:43, Timothy McDaniel: > > > > > > > > - note that the code still uses its private byte-encoded > > > > > > > > versions of > > > > > > > > umonitor/umwait, rather than the new functions in the power > > > > > > > > patch that are built on top of those intrinsics. This is > > > > > > > > intentional. > > > > > > > > > > > > > > Why? Now these intrinsics are available in the main branch. > > > > > > > We should avoid duplicating such code. > > > > > > > > > > > > > > > > > > > > > > > > > > I had asked that the low level intrinsics (UMWAIT/UMONITOR) be split > > out so > > > > > > that DLB/DLB2 could use them instead of its own private byte-encoded > > > > versions, > > > > > > but instead we have these wrappers that call the low level > > > > > > intrinsics. > > Those > > > > > > wrappers > > > > > > introduce additional overhead that is not required for DLB/DLB2. I > > > > > > have a > > > > > > meeting with Ma Liang on Monday to discuss. > > > > > > > > > > I thought the ask of DLB was to just substitute the low level > > umwait/umonitor > > > > byte > > > > > encoded instructions DLB has defined privately with similar > > > > > byte-encoded > > > > instructions defined in the power > > > > > patch. The power patch does not directly expose those, which is why I > > > > > did > > not > > > > update DLB/DLB2. > > > > > The power patch does have the advantage of centralizing the race > > avoidance > > > > > logic, which is a good thing for any PMD that wishes to take > > > > > advantage of > > > > umwait/umonitor. > > > > > > > > So you mean the overhead is a good thing? > > > > > > > > > Sorry for the confusion. I just misunderstood what was being asked of > > > > > DLB > > in > > > > regard to switching over.. That being said, > > > > > I am willing to convert DLB/DLB2 to use rte_power_monitor(...) in a > > > > > future > > > > patch-set. > > > > > > > > Why not now? > > > > > > > > Indeed there is a confusion and it looks like a lot of novlang > > > > to exit from the situation. > > > > We'll wait a clear decision with facts. > > > > > > > > > > Hi Thomas, > > > > > > I have updated DLB and DLB2 to use rte_power_monitor(...), and those > > patches are > > > ready if you are willing to accept them and the 3 power patches. > > > > > > For the sake of consistency, I see the benefit of using the power patch, > > > even if > > it is > > > slightly less efficient that the DLB-specific implementation that I > > > currently > > have. > > > We have already encountered an empty queue, so this is no longer fast path > > for the PMD. > > > > I am really concerned that the API in EAL is not the most efficient. > > Why is that? Can we improve the EAL API? > > > > From an efficiency perspective, I only noticed 2 things. > 1) The size check and associated logic. This could be avoided if we had _8, > _16, _32, _64 variants instead of 1 function that handled all possibilities, > but even that may be a wash > with branch prediction > 2) The spinlock - but this observation was my mistake, and was flawed. I was > looking at the rte_power_monitor_sync(...), and not rte_power_monitor(...), > the latter of which does not take a spinlock. > > In summary, I am no longer concerned about efficiency and suitability for > DLB/DLB2.
OK, so let's go ahead with a DLB PMD using this API.
