diff mbox series

[RFC,v4,2/4] tracing: Introduce tracepoint_is_faultable()

Message ID 20241028190927.648953-3-mathieu.desnoyers@efficios.com (mailing list archive)
State Superseded
Headers show
Series Faultable syscall tracepoints updates | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Mathieu Desnoyers Oct. 28, 2024, 7:09 p.m. UTC 
Introduce a "faultable" flag within the extended structure to know
whether a tracepoint needs rcu tasks trace grace period before reclaim.
This can be queried using tracepoint_is_faultable().

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Michael Jeanson <mjeanson@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Yonghong Song <yhs@fb.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf@vger.kernel.org
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Jordan Rife <jrife@google.com>
---
 include/linux/tracepoint-defs.h |  2 ++
 include/linux/tracepoint.h      | 24 ++++++++++++++++++++++++
 include/trace/define_trace.h    |  2 +-
 3 files changed, 27 insertions(+), 1 deletion(-)

Comments

Andrii Nakryiko Oct. 28, 2024, 8:19 p.m. UTC | #1 
On Mon, Oct 28, 2024 at 12:11 PM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> Introduce a "faultable" flag within the extended structure to know
> whether a tracepoint needs rcu tasks trace grace period before reclaim.
> This can be queried using tracepoint_is_faultable().
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Michael Jeanson <mjeanson@efficios.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Yonghong Song <yhs@fb.com>
> Cc: Paul E. McKenney <paulmck@kernel.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Cc: bpf@vger.kernel.org
> Cc: Joel Fernandes <joel@joelfernandes.org>
> Cc: Jordan Rife <jrife@google.com>
> ---
>  include/linux/tracepoint-defs.h |  2 ++
>  include/linux/tracepoint.h      | 24 ++++++++++++++++++++++++
>  include/trace/define_trace.h    |  2 +-
>  3 files changed, 27 insertions(+), 1 deletion(-)
>

LGTM

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> index 967c08d9da84..aebf0571c736 100644
> --- a/include/linux/tracepoint-defs.h
> +++ b/include/linux/tracepoint-defs.h
> @@ -32,6 +32,8 @@ struct tracepoint_func {
>  struct tracepoint_ext {
>         int (*regfunc)(void);
>         void (*unregfunc)(void);
> +       /* Flags. */
> +       unsigned int faultable:1;
>  };
>
>  struct tracepoint {
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 83dc24ee8b13..259f0ab4ece6 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -104,6 +104,12 @@ void for_each_tracepoint_in_module(struct module *mod,
>   * tracepoint_synchronize_unregister must be called between the last tracepoint
>   * probe unregistration and the end of module exit to make sure there is no
>   * caller executing a probe when it is freed.
> + *
> + * An alternative is to use the following for batch reclaim associated
> + * with a given tracepoint:
> + *
> + * - tracepoint_is_faultable() == false: call_rcu()
> + * - tracepoint_is_faultable() == true:  call_rcu_tasks_trace()
>   */
>  #ifdef CONFIG_TRACEPOINTS
>  static inline void tracepoint_synchronize_unregister(void)
> @@ -111,9 +117,17 @@ static inline void tracepoint_synchronize_unregister(void)
>         synchronize_rcu_tasks_trace();
>         synchronize_rcu();
>  }
> +static inline bool tracepoint_is_faultable(struct tracepoint *tp)
> +{
> +       return tp->ext && tp->ext->faultable;
> +}
>  #else
>  static inline void tracepoint_synchronize_unregister(void)
>  { }
> +static inline bool tracepoint_is_faultable(struct tracepoint *tp)
> +{
> +       return false;
> +}
>  #endif
>
>  #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
> @@ -345,6 +359,15 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>         struct tracepoint_ext __tracepoint_ext_##_name = {              \
>                 .regfunc = _reg,                                        \
>                 .unregfunc = _unreg,                                    \
> +               .faultable = false,                                     \
> +       };                                                              \
> +       __DEFINE_TRACE_EXT(_name, &__tracepoint_ext_##_name, PARAMS(_proto), PARAMS(_args));
> +
> +#define DEFINE_TRACE_SYSCALL(_name, _reg, _unreg, _proto, _args)       \
> +       struct tracepoint_ext __tracepoint_ext_##_name = {              \
> +               .regfunc = _reg,                                        \
> +               .unregfunc = _unreg,                                    \
> +               .faultable = true,                                      \
>         };                                                              \
>         __DEFINE_TRACE_EXT(_name, &__tracepoint_ext_##_name, PARAMS(_proto), PARAMS(_args));
>
> @@ -389,6 +412,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>  #define __DECLARE_TRACE_SYSCALL        __DECLARE_TRACE
>
>  #define DEFINE_TRACE_FN(name, reg, unreg, proto, args)
> +#define DEFINE_TRACE_SYSCALL(name, reg, unreg, proto, args)
>  #define DEFINE_TRACE(name, proto, args)
>  #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
>  #define EXPORT_TRACEPOINT_SYMBOL(name)
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
> index ff5fa17a6259..63fea2218afa 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -48,7 +48,7 @@
>
>  #undef TRACE_EVENT_SYSCALL
>  #define TRACE_EVENT_SYSCALL(name, proto, args, struct, assign, print, reg, unreg) \
> -       DEFINE_TRACE_FN(name, reg, unreg, PARAMS(proto), PARAMS(args))
> +       DEFINE_TRACE_SYSCALL(name, reg, unreg, PARAMS(proto), PARAMS(args))
>
>  #undef TRACE_EVENT_NOP
>  #define TRACE_EVENT_NOP(name, proto, args, struct, assign, print)
> --
> 2.39.5
>
Mathieu Desnoyers Oct. 28, 2024, 8:24 p.m. UTC | #2 
On 2024-10-28 16:19, Andrii Nakryiko wrote:
> On Mon, Oct 28, 2024 at 12:11 PM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> Introduce a "faultable" flag within the extended structure to know
>> whether a tracepoint needs rcu tasks trace grace period before reclaim.
>> This can be queried using tracepoint_is_faultable().
>>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Michael Jeanson <mjeanson@efficios.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Yonghong Song <yhs@fb.com>
>> Cc: Paul E. McKenney <paulmck@kernel.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
>> Cc: bpf@vger.kernel.org
>> Cc: Joel Fernandes <joel@joelfernandes.org>
>> Cc: Jordan Rife <jrife@google.com>
>> ---
>>   include/linux/tracepoint-defs.h |  2 ++
>>   include/linux/tracepoint.h      | 24 ++++++++++++++++++++++++
>>   include/trace/define_trace.h    |  2 +-
>>   3 files changed, 27 insertions(+), 1 deletion(-)
>>
> 
> LGTM

FYI I'm still missing the "static" here:


>> +#define DEFINE_TRACE_SYSCALL(_name, _reg, _unreg, _proto, _args)       \
>> +       struct tracepoint_ext __tracepoint_ext_##_name = {              \
>> +               .regfunc = _reg,                                        \
>> +               .unregfunc = _unreg,                                    \
>> +               .faultable = true,                                      \
>>          };      

Will fix in v5.

Acked-by noted, thanks!

Thanks,

Mathieu
diff mbox series

Patch

diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index 967c08d9da84..aebf0571c736 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -32,6 +32,8 @@  struct tracepoint_func {
 struct tracepoint_ext {
 	int (*regfunc)(void);
 	void (*unregfunc)(void);
+	/* Flags. */
+	unsigned int faultable:1;
 };
 
 struct tracepoint {
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 83dc24ee8b13..259f0ab4ece6 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -104,6 +104,12 @@  void for_each_tracepoint_in_module(struct module *mod,
  * tracepoint_synchronize_unregister must be called between the last tracepoint
  * probe unregistration and the end of module exit to make sure there is no
  * caller executing a probe when it is freed.
+ *
+ * An alternative is to use the following for batch reclaim associated
+ * with a given tracepoint:
+ *
+ * - tracepoint_is_faultable() == false: call_rcu()
+ * - tracepoint_is_faultable() == true:  call_rcu_tasks_trace()
  */
 #ifdef CONFIG_TRACEPOINTS
 static inline void tracepoint_synchronize_unregister(void)
@@ -111,9 +117,17 @@  static inline void tracepoint_synchronize_unregister(void)
 	synchronize_rcu_tasks_trace();
 	synchronize_rcu();
 }
+static inline bool tracepoint_is_faultable(struct tracepoint *tp)
+{
+	return tp->ext && tp->ext->faultable;
+}
 #else
 static inline void tracepoint_synchronize_unregister(void)
 { }
+static inline bool tracepoint_is_faultable(struct tracepoint *tp)
+{
+	return false;
+}
 #endif
 
 #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
@@ -345,6 +359,15 @@  static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 	struct tracepoint_ext __tracepoint_ext_##_name = {		\
 		.regfunc = _reg,					\
 		.unregfunc = _unreg,					\
+		.faultable = false,					\
+	};								\
+	__DEFINE_TRACE_EXT(_name, &__tracepoint_ext_##_name, PARAMS(_proto), PARAMS(_args));
+
+#define DEFINE_TRACE_SYSCALL(_name, _reg, _unreg, _proto, _args)	\
+	struct tracepoint_ext __tracepoint_ext_##_name = {		\
+		.regfunc = _reg,					\
+		.unregfunc = _unreg,					\
+		.faultable = true,					\
 	};								\
 	__DEFINE_TRACE_EXT(_name, &__tracepoint_ext_##_name, PARAMS(_proto), PARAMS(_args));
 
@@ -389,6 +412,7 @@  static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 #define __DECLARE_TRACE_SYSCALL	__DECLARE_TRACE
 
 #define DEFINE_TRACE_FN(name, reg, unreg, proto, args)
+#define DEFINE_TRACE_SYSCALL(name, reg, unreg, proto, args)
 #define DEFINE_TRACE(name, proto, args)
 #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
 #define EXPORT_TRACEPOINT_SYMBOL(name)
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index ff5fa17a6259..63fea2218afa 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -48,7 +48,7 @@ 
 
 #undef TRACE_EVENT_SYSCALL
 #define TRACE_EVENT_SYSCALL(name, proto, args, struct, assign, print, reg, unreg) \
-	DEFINE_TRACE_FN(name, reg, unreg, PARAMS(proto), PARAMS(args))
+	DEFINE_TRACE_SYSCALL(name, reg, unreg, PARAMS(proto), PARAMS(args))
 
 #undef TRACE_EVENT_NOP
 #define TRACE_EVENT_NOP(name, proto, args, struct, assign, print)