[PATCH] tasklet: Introduce tasklet tracepoints

[PATCH] tasklet: Introduce tasklet tracepoints

[Muchun Song]

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@bytedance.com>
---
 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;
+	),
+
+	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;
 			}
-- 
2.20.1

Re: [PATCH] tasklet: Introduce tasklet tracepoints

[Steven Rostedt]

On Sat,  5 Sep 2020 14:04:12 +0800
Muchun Song <songmuchun@bytedance.com> 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@bytedance.com>
> ---
>  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;

-- 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;
>  			}

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

[Muchun Song]

On Sat, Sep 19, 2020 at 12:46 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Sat,  5 Sep 2020 14:04:12 +0800
> Muchun Song <songmuchun@bytedance.com> 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@bytedance.com>
> > ---
> >  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

Re: [PATCH] tasklet: Introduce tasklet tracepoints

[Thomas Gleixner]

On Sat, Sep 05 2020 at 14:04, Muchun Song wrote:

> Introduce tracepoints for tasklets just like softirq does.

What does softirq?

> In this case, we can calculate tasklet latency and know what tasklet
> run.

I rather see people working on removal of tasklets.

Thanks,

        tglx

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

[Muchun Song]

On Fri, Sep 25, 2020 at 1:59 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Sat, Sep 05 2020 at 14:04, Muchun Song wrote:
>
> > Introduce tracepoints for tasklets just like softirq does.
>
> What does softirq?

I mean that the softirq has tracepoints and the tasklet may
also be needed.

>
> > In this case, we can calculate tasklet latency and know what tasklet
> > run.
>
> I rather see people working on removal of tasklets.

I do not know about this, why do we remove the tasklet?

>
> Thanks,
>
>         tglx

Thanks.

-- 
Yours,
Muchun