On 10/12/26 2:42 PM, Eelco Chaudron via dev wrote: > > > On 12 Oct 2026, at 14:36, Aaron Conole wrote: > >> Eelco Chaudron <[email protected]> writes: >> >>> On 8 Oct 2026, at 15:00, Eli Britstein via dev wrote: >>> >>>> On 06/10/2026 17:30, Eelco Chaudron wrote: >>>>> The OVS coding style requires that static inline functions defined >>>>> in .c files also be decorated with ALWAYS_INLINE. Several functions >>>>> in dpif-netdev.c and dpif-netdev-dpcls.c were missing this attribute. >>>>> Add it to every occurrence for consistency and to avoid checkpatch >>>>> warnings in later patches. >>>> >>>> Instead of forcing keeping inline by this attribute, won't it just be >>>> cleaner to remove the 'inline'? >>> >>> I included this patch, as checkpatch was complaining about the >>> new (and existing) inlines. The new one I could have added >>> without the inline keyword. I'm not comfortable removing the >>> existing entries as we have no easy way to verify the impact of >>> removing this from these files. >>> >>> Aaron, any feedback about this? >> >> Modern optimizing compilers (like gcc and clang) treat the 'inline' >> keyword as a hint - from a performance perspective. The decision to >> inline or not still sits at the level of the optimizer's call-site >> context and heuristics analysis. GCC explicitly documents this: >> https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Inline-Function-Definitions.html >> >> Just because a call can be inlined does not mean it should be >> inlined. The GNU C compiler weighs costs and benefits to decide >> whether inlining a particular call is advantageous. >> >> So it may be better to say that the previous versions are merely hinting >> that the function could be inlined - the decision is (and was) part of >> the compiler. In fact, I think it's equally likely that ALWAYS_INLINE >> can cause a performance regression rather than an improvement - >> ALWAYS_INLINE tells the compiler to ignore this cost-benefit analysis >> and just throw the code inline. The only way to know for sure would be >> to try and benchmark these calls (and I'm not sure the best way to do >> that currently - anything we did would be synthetic). We should be very >> careful about changing that hint to a forced switch. I'd be more in >> favor of dropping the 'inline' keyword usage on these static functions >> in the C file, than adding 'ALWAYS_INLINE' to their attribute list. >> That has us keep to the OVS coding convention without hampering the >> compilers ability to choose to inline the functions that it deems good >> candidates for such optimization. > > Thanks Aaron for the quick response. I will remove this patch from the series! FWIW, I'd say inlines may be important in the dpcls code and some of the PMD context and cache related functions not so sure about other parts. But I agree that we shouldn't touch this without running some performance testing.
Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
