Re: [Xen-devel] [PATCH 08/24] xen: tracing: add trace records for schedule and rate-limiting.

[Meng Xu <mengxu@xxxxxxxxxxxxx>]

On Wed, Aug 17, 2016 at 1:18 PM, Dario Faggioli
<dario.faggioli@xxxxxxxxxx> wrote:
>
> As far as {csched, csched2, rt}_schedule() are concerned,
> an "empty" event, would already make it easier to read and
> understand a trace.
>
> But while there, add a few useful information, like
> if the cpu that is going through the scheduler has
> been tickled or not, if it is currently idle, etc
> (they vary, on a per-scheduler basis).
>
> For Credit1 and Credit2, add a record about when
> rate-limiting kicks in too.
>
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> ---
> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> Cc: Meng Xu <mengxu@xxxxxxxxxxxxx>
> Cc: Anshul Makkar <anshul.makkar@xxxxxxxxxx>
> ---
>  xen/common/sched_credit.c  |    7 +++++++
>  xen/common/sched_credit2.c |   38 +++++++++++++++++++++++++++++++++++++-
>  xen/common/sched_rt.c      |   15 +++++++++++++++
>  3 files changed, 59 insertions(+), 1 deletion(-)
>


> diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
> index 41c61a7..903dbd8 100644
> --- a/xen/common/sched_rt.c
> +++ b/xen/common/sched_rt.c
> @@ -160,6 +160,7 @@
>  #define TRC_RTDS_BUDGET_BURN      TRC_SCHED_CLASS_EVT(RTDS, 3)
>  #define TRC_RTDS_BUDGET_REPLENISH TRC_SCHED_CLASS_EVT(RTDS, 4)
>  #define TRC_RTDS_SCHED_TASKLET    TRC_SCHED_CLASS_EVT(RTDS, 5)
> +#define TRC_RTDS_SCHEDULE         TRC_SCHED_CLASS_EVT(RTDS, 6)
>
>  static void repl_timer_handler(void *data);
>
> @@ -1035,6 +1036,20 @@ rt_schedule(const struct scheduler *ops, s_time_t now, 
> bool_t tasklet_work_sched
>      struct rt_vcpu *snext = NULL;
>      struct task_slice ret = { .migrated = 0 };
>
> +    /* TRACE */
> +    {
> +        struct __packed {
> +            unsigned cpu:17, tasklet:8, tickled:4, idle:4;
> +        } d;
> +        d.cpu = cpu;
> +        d.tasklet = tasklet_work_scheduled;
> +        d.tickled = cpumask_test_cpu(cpu, &prv->tickled);
> +        d.idle = is_idle_vcpu(current);
> +        __trace_var(TRC_RTDS_SCHEDULE, 1,
> +                    sizeof(d),
> +                    (unsigned char *)&d);
> +    }

I have two questions here:
1) IMHO, the trace should be wrapped by the if (
unlikely(tb_init_done) ) {} statement as done in sched_credit2.c.
Otherwise, we always enable the tracing which may hurt the
performance, I think.

2) Why does the cpu field here use 17 bits instead of 16 bits as used
in credit2?
This may be a typo I guess (since you are trying to align the
structure to 32 bits I guess ;-) )?
In addition, I'm wondering if uint16 is better than unsigned? I'm not
that confident if unsigned type will always have 16 bits on all types
of machines?

Thanks,

Meng

------------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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