Re: [PATCH 1/5] xen/domain: Remove function pointers from domain pause helpers

[Julien Grall <julien@xxxxxxx>]

 ~/works/oss/linux/scripts/bloat-o-meter xen-syms-old xen-syms

On 11/11/2021 17:57, Andrew Cooper wrote:
> Retpolines are expensive, and all these do are select between the sync and
> nosync helpers.  Pass a boolean instead, and use direct calls everywhere.
To be honest, I much prefer to read the old code. I am totally not 
against the change but I can see how I would be ready to introduce new 
function pointers use in the future.
So I think we need some guidelines on when to use function pointers in 
Xen. The more...
> Pause/unpause operations on behalf of dom0 are not fastpaths, so avoid
> exposing the __domain_pause_by_systemcontroller() internal.
>
> This actually compiles smaller than before:
... the code doesn't really compile smaller on Arm:

42sh>  ../scripts/bloat-o-meter xen-syms-old xen-syms

add/remove: 4/2 grow/shrink: 0/6 up/down: 272/-252 (20)
Function                                     old     new   delta
_domain_pause                                  -     136    +136
_domain_pause_by_systemcontroller              -     120    +120
domain_pause_by_systemcontroller_nosync        -       8      +8
domain_pause_by_systemcontroller               -       8      +8
domain_resume                                136     132      -4
domain_pause_nosync                           12       8      -4
domain_pause                                  12       8      -4
domain_pause_except_self                     188     180      -8
do_domctl                                   5480    5472      -8
domain_kill                                  372     356     -16
do_domain_pause                               88       -     -88
__domain_pause_by_systemcontroller           120       -    -120
Total: Before=966919, After=966939, chg +0.00%

>    $ ../scripts/bloat-o-meter xen-syms-before xen-syms-after
>    add/remove: 3/1 grow/shrink: 0/5 up/down: 250/-273 (-23)
>    Function                                     old     new   delta
>    _domain_pause                                  -     115    +115
>    domain_pause_by_systemcontroller               -      69     +69
>    domain_pause_by_systemcontroller_nosync        -      66     +66
>    domain_kill                                  426     398     -28
>    domain_resume                                246     214     -32
>    domain_pause_except_self                     189     141     -48
>    domain_pause                                  59      10     -49
>    domain_pause_nosync                           59       7     -52
>    __domain_pause_by_systemcontroller            64       -     -64
>
> despite GCC's best efforts.  The new _domain_pause_by_systemcontroller()
> really should not be inlined, considering that the difference is only the
> setup of the sync boolean to pass to _domain_pause(), and there are plenty of
> registers to spare.
Cheers,

--
Julien Grall