On Mon, 2019-06-03 at 22:27 +0000, Saeed Mahameed wrote:
> 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;
I experimented a bit with gcc 8.3.1 and some BP hint variations:
* with current macros we have single test for fN and an incresing
number of conditional jumps and tests for the following functions, as
the generated code looks somehow alike:
cmp f4, function_ptr
jne test_f3
call f4
post_call:
// ...
// ...
test_f3:
cmp f3, function_ptr
jne test_f2
call f3
jmp post_call
test_f2:
cmp f2, function_ptr
//...
* keeping 'likely' only on INDIRECT_CALL_1 we get a conditinal jump for
fN and the number of conditional jumps and tests grows for the next
functions, as the generated code looks somehow alike:
cmp f4, function_ptr
je call_f4
cmp f3, function_ptr
je call_f3
//...
cmp f1, function_ptr
jne indirect_call
call f1
post_call:
// ...
// ...
call_f4:
call f4
jmp post_call
call_f3:
call f3
jmp post_call
// ...
* without any BP hints, is quite alike the above, except for the last
test, the indirect call don't need an additional jump:
cmp f4, function_ptr
je call_f4
cmp f3, function_ptr
je call_f3
//...
cmp f1, function_ptr
je call_f1
call retpoline_helper
I think the first option should be overall better then the 2nd. The 3rd
one is the worse.
> In any case, just make sure to use the order i suggested in next
> patch with: MLX5_RX_INDIRECT_CALL_LIST
Sure! will do in next iteration, as soon as the above topic is settled.
Thanks!
Paolo
