On Mon, 2019-06-03 at 11:51 +0200, Paolo Abeni wrote: > On Fri, 2019-05-31 at 18:30 +0000, Saeed Mahameed wrote: > > On Fri, 2019-05-31 at 14:53 +0200, Paolo Abeni wrote: > > > Experimental results[1] has shown that resorting to several > > > branches > > > and a direct-call is faster than indirect call via retpoline, > > > even > > > when the number of added branches go up 5. > > > > > > This change adds two additional helpers, to cope with indirect > > > calls > > > with up to 4 available direct call option. We will use them > > > in the next patch. > > > > > > [1] > > > https://linuxplumbersconf.org/event/2/contributions/99/attachments/98/117/lpc18_paper_af_xdp_perf-v2.pdf > > > > > > Signed-off-by: Paolo Abeni <pab...@redhat.com> > > > --- > > > include/linux/indirect_call_wrapper.h | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/include/linux/indirect_call_wrapper.h > > > b/include/linux/indirect_call_wrapper.h > > > index 00d7e8e919c6..7c4cac87eaf7 100644 > > > --- a/include/linux/indirect_call_wrapper.h > > > +++ b/include/linux/indirect_call_wrapper.h > > > @@ -23,6 +23,16 @@ > > > likely(f == f2) ? f2(__VA_ARGS__) : > > > \ > > > INDIRECT_CALL_1(f, f1, __VA_ARGS__); > > > \ > > > }) > > > +#define INDIRECT_CALL_3(f, f3, f2, f1, ...) > > > > > > \ > > > + ({ > > > \ > > > + likely(f == f3) ? f3(__VA_ARGS__) : > > > \ > > > + INDIRECT_CALL_2(f, f2, f1, > > > __VA_ARGS__); \ > > > + }) > > > +#define INDIRECT_CALL_4(f, f4, f3, f2, f1, ...) > > > \ > > > + ({ > > > \ > > > + likely(f == f4) ? f4(__VA_ARGS__) : > > > > do we really want "likely" here ? in our cases there is no > > preference > > on whuch fN is going to have the top priority, all of them are > > equally > > important and statically configured and guranteed to not change on > > data > > path .. > > I was a little undecided about that, too. 'likely()' is there mainly > for simmetry with the already existing _1 and _2 variants. In such > macros the branch prediction hint represent a real priority of the > available choices. >
For macro _1 it make sense to have the likely keyword but for _2 it doesn't, by looking at most of the usecases of INDIRECT_CALL_2, they seem to be all around improving tcp/udp related indirection calls in the protocol stack, and they seem to prefer tcp over udp. But IMHO at least for the above usecase I think the likely keyword is being misused here and should be remove from all INDIRECT_CALL_N where N > 1; Eric, what do you think ? > To avoid the branch prediction, a new set of macros should be > defined, > but that also sounds redundant. > > If you have strong opinion against the breanch prediction hint, I > could > either drop this patch and the next one or resort to custom macros in > the mlx code. > > Any [alternative] suggestions more than welcome! > \ custom macros can work, but in case you don't want to introduce such macros in a vendor specific driver, then i think your patches are still an improvement after all.. In any case, just make sure to use the order i suggested in next patch with: MLX5_RX_INDIRECT_CALL_LIST > > > + INDIRECT_CALL_3(f, f3, f2, f1, > > > __VA_ARGS__); \ > > > + }) > > > > > > > Oh the RETPOLINE! > > > > On which (N) where INDIRECT_CALL_N(f, fN, fN-1, ..., f1,...) , > > calling > > the indirection function pointer directly is going to be actually > > better than this whole INDIRECT_CALL_N wrapper "if else" dance ? > > In commit ce02ef06fcf7a399a6276adb83f37373d10cbbe1, it's measured a > relevant gain even with more than 5 options. I personally would avoid > adding much more options than the above. > > Thanks, > > Paolo > >
