Re: [PATCH V3 07/11] notifiers: Add tracepoints to the notifiers infrastructure

2022-11-22 Thread Guilherme G. Piccoli
On 19/08/2022 19:17, Guilherme G. Piccoli wrote:
> 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 
> Cc: Cong Wang 
> Cc: Sebastian Andrzej Siewior 
> Cc: Steven Rostedt 
> Cc: Valentin Schneider 
> Cc: Xiaoming Ni 
> Signed-off-by: Guilherme G. Piccoli 
> 
> ---
> 
> V3:
> - Yet another major change - thanks to Arjan's great suggestion,
> refactored the code to make use of tracepoints instead of guarding
> the output with 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.
> 
> 
>  include/trace/events/notifiers.h | 69 
>  kernel/notifier.c|  6 +++
>  2 files changed, 75 insertions(+)
>  create mode 100644 include/trace/events/notifiers.h
> 

Hi Arjan / Xiaoming / all, monthly ping heh

If there's any advice on how to move things here, I would appreciate!
Thanks in advance and apologies for the re-pings.

Cheers,


Guilherme

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH V3 07/11] notifiers: Add tracepoints to the notifiers infrastructure

2022-10-21 Thread Guilherme G. Piccoli
On 19/08/2022 19:17, Guilherme G. Piccoli wrote:
> 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 
> Cc: Cong Wang 
> Cc: Sebastian Andrzej Siewior 
> Cc: Steven Rostedt 
> Cc: Valentin Schneider 
> Cc: Xiaoming Ni 
> Signed-off-by: Guilherme G. Piccoli 
> 
> ---
> 
> V3:
> - Yet another major change - thanks to Arjan's great suggestion,
> refactored the code to make use of tracepoints instead of guarding
> the output with 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.
> 

Hi Arjan / all, apologies for the re-ping.
Did you have a chance to take a look in this one, is there anything else
I could improve?

Thanks in advance,


Guilherme

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH V3 07/11] notifiers: Add tracepoints to the notifiers infrastructure

2022-09-19 Thread Guilherme G. Piccoli
On 19/08/2022 19:17, Guilherme G. Piccoli wrote:
> 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 
> Cc: Cong Wang 
> Cc: Sebastian Andrzej Siewior 
> Cc: Steven Rostedt 
> Cc: Valentin Schneider 
> Cc: Xiaoming Ni 
> Signed-off-by: Guilherme G. Piccoli 
> 
> ---
> 
> V3:
> - Yet another major change - thanks to Arjan's great suggestion,
> refactored the code to make use of tracepoints instead of guarding
> the output with 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.
> 
> [...]

Hi Arjan / Xioming, apologies for the ping.
Do you think the patch is good enough now? I liked the tracepoint
approach, indeed it was a much better idea than guarding the prints with
the DEBUG Kconfig heheh

Appreciate your opinions!
Thanks in advance,


Guilherme

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH V3 07/11] notifiers: Add tracepoints to the notifiers infrastructure

2022-08-24 Thread 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 
Cc: Cong Wang 
Cc: Sebastian Andrzej Siewior 
Cc: Steven Rostedt 
Cc: Valentin Schneider 
Cc: Xiaoming Ni 
Signed-off-by: Guilherme G. Piccoli 

---

V3:
- Yet another major change - thanks to Arjan's great suggestion,
refactored the code to make use of tracepoints instead of guarding
the output with 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.


 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 ..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 
+
+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 
diff --git a/kernel/notifier.c b/kernel/notifier.c
index 0d5bd62c480e..2f2783f59a31 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -7,6 +7,9 @@
 #include 
 #include 
 
+#define CREATE_TRACE_POINTS
+#include 
+
 /*
  * 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.37.2


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec