Re: [Xen-devel] [PATCH 02/15] xen: tracing: avoid checking tb_init_done multiple times.

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 01/06/17 18:33, Dario Faggioli wrote:
> In fact, when calling __trace_var() directly, we can
> assume that tb_init_done has been checked to be true,
> and the if is hence redundant.
>
> While there, also:
>  - still in __trace_var(), move the check that the event
>    is actually being traced up a little bit (to bail as
>    soon as possible, if it is not);
>  - make it explicit that tb_init_done is likely 0 in
>    trace_will_trace_event(), as it is almost everywhere
>    in the code.
>
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> ---
> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> ---
>  xen/common/trace.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/xen/common/trace.c b/xen/common/trace.c
> index 4fedc26..f29cd4c 100644
> --- a/xen/common/trace.c
> +++ b/xen/common/trace.c
> @@ -311,7 +311,7 @@ static int tb_set_size(unsigned int pages)
>  
>  int trace_will_trace_event(u32 event)
>  {
> -    if ( !tb_init_done )
> +    if ( likely(!tb_init_done) )
>          return 0;
>  
>      /*
> @@ -691,7 +691,8 @@ void __trace_var(u32 event, bool_t cycles, unsigned int 
> extra,
>      unsigned int extra_word;
>      bool_t started_below_highwater;

You probably want an ASSERT(tb_init_done) here to keep callers in check.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

>  
> -    if( !tb_init_done )
> +    /* If the event is not interesting, bail, as early as possible */
> +    if ( (tb_event_mask & event) == 0 )
>          return;
>  
>      /* Convert byte count into word count, rounding up */
> @@ -705,9 +706,6 @@ void __trace_var(u32 event, bool_t cycles, unsigned int 
> extra,
>      /* Round size up to nearest word */
>      extra = extra_word * sizeof(u32);
>  
> -    if ( (tb_event_mask & event) == 0 )
> -        return;
> -
>      /* match class */
>      if ( ((tb_event_mask >> TRC_CLS_SHIFT) & (event >> TRC_CLS_SHIFT)) == 0 )
>          return;
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel