Re: [PATCH 08/15] xen: trace RCU behavior

[George Dunlap <george.dunlap@citrix.com>]

On 01/06/17 18:34, Dario Faggioli wrote:
> Making it possible generate events showing the
> activity and the behavior of the RCU subsystem.
> 
> Gate this with its specific Kconfig option (under
> CONFIG_TRACING), and keep it in disabled state by
> default.
> ---
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> ---
>  xen/Kconfig.debug          |    8 ++++
>  xen/common/rcupdate.c      |   82 ++++++++++++++++++++++++++++++++++++++++----
>  xen/include/public/trace.h |   14 ++++++++
>  3 files changed, 97 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index 81910c9..fd5cccc 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -116,6 +116,14 @@ config TRACE_IRQSOFF
>            Note that this comes with high overead and produces huge amount of
>            tracing data.
>  
> +config TRACE_RCU
> +       bool "Trace RCU behavior" if EXPERT = "y"
> +       default n
> +       depends on TRACING
> +       ---help---
> +         Makes it possible generate events  showing the activity and the
> +         behavior of the RCU subsystem.
> +
>  config VERBOSE_DEBUG
>  	bool "Verbose debug messages"
>  	default DEBUG
> diff --git a/xen/common/rcupdate.c b/xen/common/rcupdate.c
> index 8cc5a82..f160582 100644
> --- a/xen/common/rcupdate.c
> +++ b/xen/common/rcupdate.c
> @@ -92,6 +92,57 @@ static int qhimark = 10000;
>  static int qlowmark = 100;
>  static int rsinterval = 1000;
>  
> +#ifdef CONFIG_TRACE_RCU
> +static inline void trace_call_rcu(void *func)
> +{
> +    uint64_t addr = (uint64_t)func;
> +
> +    if ( likely(!tb_init_done) )
> +        return;
> +
> +    __trace_var(TRC_XEN_RCU_CALL_RCU, 0, sizeof(addr), &addr);
> +}
> +static inline void trace_start_batch(const cpumask_t *m)
> +{
> +    uint32_t mask[6];
> +
> +    if ( likely(!tb_init_done) )
> +        return;
> +
> +    memset(mask, 0, sizeof(mask));
> +    memcpy(mask, m, min(sizeof(mask), sizeof(cpumask_t)));
> +    __trace_var(TRC_XEN_RCU_START_BATCH, 0, sizeof(mask), &mask);

Another possibility here would be to say something like:

size = min(sizeof(mask), sizeof(cpumask_t))
memcpy(mask, m, size)
__trace_var(..., 0, size, &mask)

That would result in a more efficient log on systems which, say, had
defied NR_CPUS to something relatively small for efficiency reasons; and
you wouldn't need to clear the unused bits.

Everything else looks good (with Jan's comments).

 -George


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