Re: [RFC v2] eal: provide option to use compiler memcpy instead of RTE

["Mattias Rönnblom" <hofors@lysator.liu.se>]

On 2024-05-31 18:50, Stephen Hemminger wrote:
> On Fri, 31 May 2024 07:19:41 +0200
> Mattias Rönnblom <hofors@lysator.liu.se> wrote:
> 
>> On 2024-05-28 17:09, Bruce Richardson wrote:
>>> On Tue, May 28, 2024 at 07:59:36AM -0700, Stephen Hemminger wrote:
>>>> On Tue, 28 May 2024 10:19:15 +0200
>>>> Mattias Rönnblom <hofors@lysator.liu.se> wrote:
>>>>   
>>>>>>       
>>>>>
>>>>> I've tested this patch some with DSW micro benchmarks, and the result is
>>>>> a 2.5% reduction of the DSW+testapp overhead with cc/libc memcpy. GCC 11.4.
>>>>>
>>>>> We've also run characteristic test suite of a large, real world app.
>>>>> Here, we saw no effect. GCC 10.5.
>>>>>
>>>>> x86_64 in both cases (Skylake and Raptor Lake).
>>>>>
>>>>> Last time we did the same, there were a noticeable performance
>>>>> degradation in both the above cases.
>>>>>
>>>>> This is not a lot of data points, but I think it we should consider
>>>>> making the custom RTE memcpy() implementations optional in the next
>>>>> release, and if no-one complains, remove the implementations in the next
>>>>> release.
>>>>
>>>> Lets go farther.
>>>>
>>>> 1. Announce that rte_memcpy will be marked deprecated in 24.11 release
>>>>
>>>> 2. In 24.11 do a global replace of rte_memcpy on the tree.
>>>>      And mark rte_memcpy as deprecated.
>>>>
>>>> 3. In 25.11 it can go away.
>>>
>>> While I'd like us to be able to do so, I believe that to be premature. We
>>> need to see where/if there are regressions first, and see about fixing
>>> them.
>>>
>>> /Bruce
>>
>> Should I turn this RFC into a PATCH?
>>
>> Is use_cc_memcpy a good name for the configuration parameter?
>>
> 
> I did a slightly more direct test and found a couple of things:
>     1. Ena driver is redefining memcpy as rte_memcpy, this should be removed and should have
>        been blocked during code review.

Wouldn't that hack continue to work? Provided rte_memcpy() is a 
function, and the <rte_memcpy.h> header is included prior to the memcpy 
redefinition.

>     2. A couple of drivers are implicitly expecting simd vector routines to be available.
>        This works because rte_memcpy.h includes rte_vect.h.  The fix is to have these
>        places include rte_vect.h
> 

I noticed this as well. I'll add patches for those drivers.