lkml.org 
[lkml]   [2020]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Sched: Add a tracepoint to track rq->nr_running
On Fri, Jun 19, 2020 at 12:46:41PM -0400 Steven Rostedt wrote:
> On Fri, 19 Jun 2020 10:11:20 -0400
> Phil Auld <pauld@redhat.com> wrote:
>
> >
> > diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
> > index ed168b0e2c53..a6d9fe5a68cf 100644
> > --- a/include/trace/events/sched.h
> > +++ b/include/trace/events/sched.h
> > @@ -634,6 +634,10 @@ DECLARE_TRACE(sched_overutilized_tp,
> > TP_PROTO(struct root_domain *rd, bool overutilized),
> > TP_ARGS(rd, overutilized));
> >
> > +DECLARE_TRACE(sched_update_nr_running_tp,
> > + TP_PROTO(int cpu, int change, unsigned int nr_running),
> > + TP_ARGS(cpu, change, nr_running));
> > +
> > #endif /* _TRACE_SCHED_H */
> >
> > /* This part must be outside protection */
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 9a2fbf98fd6f..6f28fdff1d48 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -6,7 +6,10 @@
> > *
> > * Copyright (C) 1991-2002 Linus Torvalds
> > */
> > +
> > +#define SCHED_CREATE_TRACE_POINTS
> > #include "sched.h"
> > +#undef SCHED_CREATE_TRACE_POINTS
>
> Because of the macro magic, and really try not to have trace events
> defined in any headers. Otherwise, we have weird defines like you are
> doing, and it doesn't fully protect it if a C file adds this header and
> defines CREATE_TRACE_POINTS first.
>
>
> >
> > #include <linux/nospec.h>
> >
> > @@ -21,9 +24,6 @@
> >
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -75,6 +75,15 @@
> > #include "cpupri.h"
> > #include "cpudeadline.h"
> >
> > +#ifdef SCHED_CREATE_TRACE_POINTS
> > +#define CREATE_TRACE_POINTS
> > +#endif
> > +#include <trace/events/sched.h>
> > +
> > +#ifdef SCHED_CREATE_TRACE_POINTS
> > +#undef CREATE_TRACE_POINTS
> > +#endif
> > +
> > #ifdef CONFIG_SCHED_DEBUG
> > # define SCHED_WARN_ON(x) WARN_ONCE(x, #x)
> > #else
> > @@ -1959,6 +1968,7 @@ static inline void add_nr_running(struct rq *rq, unsigned count)
> > unsigned prev_nr = rq->nr_running;
> >
> > rq->nr_running = prev_nr + count;
> > + trace_sched_update_nr_running_tp(cpu_of(rq), count, rq->nr_running);
>
> Instead of having sched.h define CREATE_TRACE_POINTS, I would have the
> following:
>
> if (trace_sched_update_nr_running_tp_enabled()) {
> call_trace_sched_update_nr_runnig(rq, count);
> }
>
> Then in sched/core.c:
>
> void trace_sched_update_nr_running(struct rq *rq, int count)
> {
> trace_sched_update_nr_running_tp(cpu_of(rq), count, rq->nr_running);
> }
>
> The trace_*_enabled() above uses static branches, where the if turns to
> a nop (pass through) when disabled and a jmp when enabled (same logic
> that trace points use themselves).
>
> Then you don't need this macro dance, and risk having another C file
> define CREATE_TRACE_POINTS and spend hours debugging why it suddenly
> broke.
>

Awesome, thanks Steve. I was really hoping there was a better way to do
that. I try it this way.


Cheers,
Phil

> -- Steve
>

--

\
 
 \ /
  Last update: 2020-06-19 19:35    [W:0.126 / U:0.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site