[PATCH v4] notifiers: Add tracepoints to the notifiers infrastructure

[PATCH v4] notifiers: Add tracepoints to the notifiers infrastructure

[Guilherme G. Piccoli]

Currently there is no way to show the callback names for registered,
unregistered or executed notifiers. This is very useful for debug
purposes, hence add this functionality here in the form of notifiers'
tracepoints, one per operation.

Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Michael Kelley <mikelley@microsoft.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Xiaoming Ni <nixiaoming@huawei.com>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
---

V4:
- Rebased and tested on top of v6.3-rc2.
- Sending as standalone patch, not part of any series.

V3:
- Yet another major change - thanks to Arjan's great suggestion,
refactored the code to make use of tracepoints instead of guarding
the output under a Kconfig debug setting.

V2:
- Major improvement thanks to the great idea from Xiaoming - changed
all the ksym wheel reinvention to printk %ps modifier;

- Instead of ifdefs, using IS_ENABLED() - thanks Steven.

- Removed an unlikely() hint on debug path.

V3: https://lore.kernel.org/lkml/20220819221731.480795-8-gpiccoli@igalia.com/
V2: https://lore.kernel.org/lkml/20220719195325.402745-10-gpiccoli@igalia.com/

Thanks in advance for reviews!
Cheers,

Guilherme


 include/trace/events/notifiers.h | 69 ++++++++++++++++++++++++++++++++
 kernel/notifier.c                |  6 +++
 2 files changed, 75 insertions(+)
 create mode 100644 include/trace/events/notifiers.h

diff --git a/include/trace/events/notifiers.h b/include/trace/events/notifiers.h
new file mode 100644
index 000000000000..e8f30631aef5
--- /dev/null
+++ b/include/trace/events/notifiers.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM notifiers
+
+#if !defined(_TRACE_NOTIFIERS_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_NOTIFIERS_H
+
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(notifiers_info,
+
+	TP_PROTO(void *cb),
+
+	TP_ARGS(cb),
+
+	TP_STRUCT__entry(
+		__field(void *, cb)
+	),
+
+	TP_fast_assign(
+		__entry->cb = cb;
+	),
+
+	TP_printk("%ps", __entry->cb)
+);
+
+/*
+ * notifiers_register - called upon notifier callback registration
+ *
+ * @cb:		callback pointer
+ *
+ */
+DEFINE_EVENT(notifiers_info, notifiers_register,
+
+	TP_PROTO(void *cb),
+
+	TP_ARGS(cb)
+);
+
+/*
+ * notifiers_unregister - called upon notifier callback unregistration
+ *
+ * @cb:		callback pointer
+ *
+ */
+DEFINE_EVENT(notifiers_info, notifiers_unregister,
+
+	TP_PROTO(void *cb),
+
+	TP_ARGS(cb)
+);
+
+/*
+ * notifiers_run - called upon notifier callback execution
+ *
+ * @cb:		callback pointer
+ *
+ */
+DEFINE_EVENT(notifiers_info, notifiers_run,
+
+	TP_PROTO(void *cb),
+
+	TP_ARGS(cb)
+);
+
+#endif /* _TRACE_NOTIFIERS_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/kernel/notifier.c b/kernel/notifier.c
index d353e4b5402d..f57e54ddbb5f 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -7,6 +7,9 @@
 #include <linux/vmalloc.h>
 #include <linux/reboot.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/notifiers.h>
+
 /*
  *	Notifier list for kernel code which wants to be called
  *	at shutdown. This is used to stop any idling DMA operations
@@ -37,6 +40,7 @@ static int notifier_chain_register(struct notifier_block **nl,
 	}
 	n->next = *nl;
 	rcu_assign_pointer(*nl, n);
+	trace_notifiers_register((void*)n->notifier_call);
 	return 0;
 }
 
@@ -46,6 +50,7 @@ static int notifier_chain_unregister(struct notifier_block **nl,
 	while ((*nl) != NULL) {
 		if ((*nl) == n) {
 			rcu_assign_pointer(*nl, n->next);
+			trace_notifiers_unregister((void*)n->notifier_call);
 			return 0;
 		}
 		nl = &((*nl)->next);
@@ -84,6 +89,7 @@ static int notifier_call_chain(struct notifier_block **nl,
 			continue;
 		}
 #endif
+		trace_notifiers_run((void*)nb->notifier_call);
 		ret = nb->notifier_call(nb, val, v);
 
 		if (nr_calls)
-- 
2.39.2

Re: [PATCH v4] notifiers: Add tracepoints to the notifiers infrastructure

[Andrew Morton]

On Tue, 14 Mar 2023 17:00:58 -0300 "Guilherme G. Piccoli" <gpiccoli@igalia.com> wrote:

>  include/trace/events/notifiers.h | 69 ++++++++++++++++++++++++++++++++
>  kernel/notifier.c                |  6 +++

Perhaps the filenames should match, which means "notifier.h".

Re: [PATCH v4] notifiers: Add tracepoints to the notifiers infrastructure

[Guilherme G. Piccoli]

On 14/03/2023 17:50, Andrew Morton wrote:
> On Tue, 14 Mar 2023 17:00:58 -0300 "Guilherme G. Piccoli" <gpiccoli@igalia.com> wrote:
> 
>>  include/trace/events/notifiers.h | 69 ++++++++++++++++++++++++++++++++
>>  kernel/notifier.c                |  6 +++
> 
> Perhaps the filenames should match, which means "notifier.h".

Hi Andrew, thanks!

Do you want me to re-submit? I see some emails of the patch getting
added to "mm-nonmm-unstable" (and also a checkpatch fixes you added on
top of that). Lemme know how should I proceed.

Cheers,


Guilherme

Re: [PATCH v4] notifiers: Add tracepoints to the notifiers infrastructure

[Andrew Morton]

On Tue, 14 Mar 2023 18:08:37 -0300 "Guilherme G. Piccoli" <gpiccoli@igalia.com> wrote:

> On 14/03/2023 17:50, Andrew Morton wrote:
> > On Tue, 14 Mar 2023 17:00:58 -0300 "Guilherme G. Piccoli" <gpiccoli@igalia.com> wrote:
> > 
> >>  include/trace/events/notifiers.h | 69 ++++++++++++++++++++++++++++++++
> >>  kernel/notifier.c                |  6 +++
> > 
> > Perhaps the filenames should match, which means "notifier.h".
> 
> Hi Andrew, thanks!
> 
> Do you want me to re-submit? I see some emails of the patch getting
> added to "mm-nonmm-unstable" (and also a checkpatch fixes you added on
> top of that). Lemme know how should I proceed.

I changed it, thanks.

Re: [PATCH v4] notifiers: Add tracepoints to the notifiers infrastructure

[Guilherme G. Piccoli]

On 14/03/2023 18:18, Andrew Morton wrote:
> On Tue, 14 Mar 2023 18:08:37 -0300 "Guilherme G. Piccoli" <gpiccoli@igalia.com> wrote:
> [...]
>> Hi Andrew, thanks!
>>
>> Do you want me to re-submit? I see some emails of the patch getting
>> added to "mm-nonmm-unstable" (and also a checkpatch fixes you added on
>> top of that). Lemme know how should I proceed.
> 
> I changed it, thanks.

Tnx a bunch =)