Re: [External] Re: [PATCH] tasklet: Introduce tasklet tracepoints

[Muchun Song]

On Sat, Sep 19, 2020 at 12:46 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Sat,  5 Sep 2020 14:04:12 +0800
> Muchun Song <songmuchun@xxxxxxxxxxxxx> wrote:
>
> > Introduce tracepoints for tasklets just like softirq does. In this case,
> > we can calculate tasklet latency and know what tasklet run.
> >
> > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> > ---
> >  include/trace/events/irq.h | 44 ++++++++++++++++++++++++++++++++++++++
> >  kernel/softirq.c           |  2 ++
> >  2 files changed, 46 insertions(+)
> >
> > diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
> > index eeceafaaea4c..69a16f3a21c2 100644
> > --- a/include/trace/events/irq.h
> > +++ b/include/trace/events/irq.h
> > @@ -160,6 +160,50 @@ DEFINE_EVENT(softirq, softirq_raise,
> >       TP_ARGS(vec_nr)
> >  );
> >
> > +DECLARE_EVENT_CLASS(tasklet,
> > +
> > +     TP_PROTO(struct tasklet_struct *t),
> > +
> > +     TP_ARGS(t),
> > +
> > +     TP_STRUCT__entry(
> > +             __field(        void *, callback        )
> > +     ),
> > +
> > +     TP_fast_assign(
> > +             __entry->callback = t->callback;
>
> I think you need to replicate the logic in the code:
>
>                 if (t->use_callback)
>                         __entry->callback = t->callback;
>                 else
>                         __entry->callback = t->func;

The `callback` and `func` is union and `callback` will replace
`func` someday in the feature. So I think that here we use
`t->callback` is enough. Right? Thanks.

>
> -- Steve
>
> > +     ),
> > +
> > +     TP_printk("callback=%ps", __entry->callback)
> > +);
> > +
> > +/**
> > + * tasklet_entry - called immediately before the tasklet handler
> > + * @t: pointer to struct tasklet_struct
> > + *
> > + * When used in combination with the tasklet_exit tracepoint
> > + * we can determine the tasklet handler routine.
> > + */
> > +DEFINE_EVENT(tasklet, tasklet_entry,
> > +
> > +     TP_PROTO(struct tasklet_struct *t),
> > +
> > +     TP_ARGS(t)
> > +);
> > +
> > +/**
> > + * tasklet_exit - called immediately after the tasklet handler returns
> > + * @t: pointer to struct tasklet_struct
> > + *
> > + * When used in combination with the tasklet_entry tracepoint
> > + * we can determine the tasklet handler routine.
> > + */
> > +DEFINE_EVENT(tasklet, tasklet_exit,
> > +
> > +     TP_PROTO(struct tasklet_struct *t),
> > +
> > +     TP_ARGS(t)
> > +);
> >  #endif /*  _TRACE_IRQ_H */
> >
> >  /* This part must be outside protection */
> > diff --git a/kernel/softirq.c b/kernel/softirq.c
> > index bf88d7f62433..0f9f5b2cc3d3 100644
> > --- a/kernel/softirq.c
> > +++ b/kernel/softirq.c
> > @@ -553,10 +553,12 @@ static void tasklet_action_common(struct softirq_action *a,
> >                               if (!test_and_clear_bit(TASKLET_STATE_SCHED,
> >                                                       &t->state))
> >                                       BUG();
> > +                             trace_tasklet_entry(t);
> >                               if (t->use_callback)
> >                                       t->callback(t);
> >                               else
> >                                       t->func(t->data);
> > +                             trace_tasklet_exit(t);
> >                               tasklet_unlock(t);
> >                               continue;
> >                       }
>


-- 
Yours,
Muchun