Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1727011 > unrolled thread

[PATCH v2 25/40] tracing: Add support for dynamic tracepoints

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 25/40] tracing: Add support for dynamic tracepoints Tom Zanussi <tom.zanussi@linux.intel.com> - 2017-09-06 00:00 +0200
    Re: [PATCH v2 25/40] tracing: Add support for dynamic tracepoints Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-09-06 01:30 +0200
      Re: [PATCH v2 25/40] tracing: Add support for dynamic tracepoints Tom Zanussi <tom.zanussi@linux.intel.com> - 2017-09-06 04:40 +0200
    Re: [PATCH v2 25/40] tracing: Add support for dynamic tracepoints Steven Rostedt <rostedt@goodmis.org> - 2017-09-08 00:10 +0200
      Re: [PATCH v2 25/40] tracing: Add support for dynamic tracepoints Tom Zanussi <tom.zanussi@linux.intel.com> - 2017-09-08 16:20 +0200

#1727011 — [PATCH v2 25/40] tracing: Add support for dynamic tracepoints

The tracepoint infrastructure assumes statically-defined tracepoints
and uses static_keys for tracepoint enablement.  In order to define
tracepoints on the fly, we need to have a dynamic counterpart.

Add a 'dynamic' flag to struct tracepoint along with accompanying
logic for this purpose.

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
---
 include/linux/tracepoint-defs.h |  1 +
 kernel/tracepoint.c             | 18 +++++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index a031920..bc22d54 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -32,6 +32,7 @@ struct tracepoint {
 	int (*regfunc)(void);
 	void (*unregfunc)(void);
 	struct tracepoint_func __rcu *funcs;
+	bool dynamic;
 };
 
 #endif
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 685c50a..1c5957f 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -197,7 +197,9 @@ static int tracepoint_add_func(struct tracepoint *tp,
 	struct tracepoint_func *old, *tp_funcs;
 	int ret;
 
-	if (tp->regfunc && !static_key_enabled(&tp->key)) {
+	if (tp->regfunc &&
+	    ((tp->dynamic && !(atomic_read(&tp->key.enabled) > 0)) ||
+	     !static_key_enabled(&tp->key))) {
 		ret = tp->regfunc();
 		if (ret < 0)
 			return ret;
@@ -219,7 +221,9 @@ static int tracepoint_add_func(struct tracepoint *tp,
 	 * is used.
 	 */
 	rcu_assign_pointer(tp->funcs, tp_funcs);
-	if (!static_key_enabled(&tp->key))
+	if (tp->dynamic && !(atomic_read(&tp->key.enabled) > 0))
+		atomic_inc(&tp->key.enabled);
+	else if (!tp->dynamic && !static_key_enabled(&tp->key))
 		static_key_slow_inc(&tp->key);
 	release_probes(old);
 	return 0;
@@ -246,10 +250,14 @@ static int tracepoint_remove_func(struct tracepoint *tp,
 
 	if (!tp_funcs) {
 		/* Removed last function */
-		if (tp->unregfunc && static_key_enabled(&tp->key))
+		if (tp->unregfunc &&
+		    ((tp->dynamic && (atomic_read(&tp->key.enabled) > 0)) ||
+		     static_key_enabled(&tp->key)))
 			tp->unregfunc();
 
-		if (static_key_enabled(&tp->key))
+		if (tp->dynamic && (atomic_read(&tp->key.enabled) > 0))
+			atomic_dec(&tp->key.enabled);
+		else if (!tp->dynamic && static_key_enabled(&tp->key))
 			static_key_slow_dec(&tp->key);
 	}
 	rcu_assign_pointer(tp->funcs, tp_funcs);
@@ -258,7 +266,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
 }
 
 /**
- * tracepoint_probe_register -  Connect a probe to a tracepoint
+ * tracepoint_probe_register_prio -  Connect a probe to a tracepoint
  * @tp: tracepoint
  * @probe: probe handler
  * @data: tracepoint data
-- 
1.9.3

[toc] | [next] | [standalone]

#1727076

----- On Sep 5, 2017, at 5:57 PM, Tom Zanussi tom.zanussi@linux.intel.com wrote:

> The tracepoint infrastructure assumes statically-defined tracepoints
> and uses static_keys for tracepoint enablement.  In order to define
> tracepoints on the fly, we need to have a dynamic counterpart.
> 
> Add a 'dynamic' flag to struct tracepoint along with accompanying
> logic for this purpose.
> 
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> ---
> include/linux/tracepoint-defs.h |  1 +
> kernel/tracepoint.c             | 18 +++++++++++++-----
> 2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> index a031920..bc22d54 100644
> --- a/include/linux/tracepoint-defs.h
> +++ b/include/linux/tracepoint-defs.h
> @@ -32,6 +32,7 @@ struct tracepoint {
> 	int (*regfunc)(void);
> 	void (*unregfunc)(void);
> 	struct tracepoint_func __rcu *funcs;
> +	bool dynamic;
> };
> 
> #endif
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 685c50a..1c5957f 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -197,7 +197,9 @@ static int tracepoint_add_func(struct tracepoint *tp,
> 	struct tracepoint_func *old, *tp_funcs;
> 	int ret;
> 
> -	if (tp->regfunc && !static_key_enabled(&tp->key)) {
> +	if (tp->regfunc &&
> +	    ((tp->dynamic && !(atomic_read(&tp->key.enabled) > 0)) ||
> +	     !static_key_enabled(&tp->key))) {
> 		ret = tp->regfunc();
> 		if (ret < 0)
> 			return ret;
> @@ -219,7 +221,9 @@ static int tracepoint_add_func(struct tracepoint *tp,
> 	 * is used.
> 	 */
> 	rcu_assign_pointer(tp->funcs, tp_funcs);
> -	if (!static_key_enabled(&tp->key))
> +	if (tp->dynamic && !(atomic_read(&tp->key.enabled) > 0))
> +		atomic_inc(&tp->key.enabled);
> +	else if (!tp->dynamic && !static_key_enabled(&tp->key))
> 		static_key_slow_inc(&tp->key);
> 	release_probes(old);
> 	return 0;
> @@ -246,10 +250,14 @@ static int tracepoint_remove_func(struct tracepoint *tp,
> 
> 	if (!tp_funcs) {
> 		/* Removed last function */
> -		if (tp->unregfunc && static_key_enabled(&tp->key))
> +		if (tp->unregfunc &&
> +		    ((tp->dynamic && (atomic_read(&tp->key.enabled) > 0)) ||
> +		     static_key_enabled(&tp->key)))
> 			tp->unregfunc();
> 
> -		if (static_key_enabled(&tp->key))
> +		if (tp->dynamic && (atomic_read(&tp->key.enabled) > 0))
> +			atomic_dec(&tp->key.enabled);
> +		else if (!tp->dynamic && static_key_enabled(&tp->key))
> 			static_key_slow_dec(&tp->key);
> 	}
> 	rcu_assign_pointer(tp->funcs, tp_funcs);
> @@ -258,7 +266,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
> }
> 
> /**
> - * tracepoint_probe_register -  Connect a probe to a tracepoint
> + * tracepoint_probe_register_prio -  Connect a probe to a tracepoint
>  * @tp: tracepoint
>  * @probe: probe handler
>  * @data: tracepoint data

Hi Tom,

Thanks for updating your approach to dynamic tracepoints.

Since you're fixing up this comment above tracepoint_probe_register_prio,
can you also remove the following line above tracepoint_probe_register
while you are at it ?

 * @prio: priority of this function over other registered functions

only the tracepoint_probe_register_prio function has the "prio" parameter.

Thanks!

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

[toc] | [prev] | [next] | [standalone]

#1727118

Hi Mathieu,

On Tue, 2017-09-05 at 23:29 +0000, Mathieu Desnoyers wrote:
> ----- On Sep 5, 2017, at 5:57 PM, Tom Zanussi tom.zanussi@linux.intel.com wrote:
> 
> > The tracepoint infrastructure assumes statically-defined tracepoints
> > and uses static_keys for tracepoint enablement.  In order to define
> > tracepoints on the fly, we need to have a dynamic counterpart.
> > 
> > Add a 'dynamic' flag to struct tracepoint along with accompanying
> > logic for this purpose.
> > 
> > Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> > ---
> > include/linux/tracepoint-defs.h |  1 +
> > kernel/tracepoint.c             | 18 +++++++++++++-----
> > 2 files changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> > index a031920..bc22d54 100644
> > --- a/include/linux/tracepoint-defs.h
> > +++ b/include/linux/tracepoint-defs.h
> > @@ -32,6 +32,7 @@ struct tracepoint {
> > 	int (*regfunc)(void);
> > 	void (*unregfunc)(void);
> > 	struct tracepoint_func __rcu *funcs;
> > +	bool dynamic;
> > };
> > 
> > #endif
> > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > index 685c50a..1c5957f 100644
> > --- a/kernel/tracepoint.c
> > +++ b/kernel/tracepoint.c
> > @@ -197,7 +197,9 @@ static int tracepoint_add_func(struct tracepoint *tp,
> > 	struct tracepoint_func *old, *tp_funcs;
> > 	int ret;
> > 
> > -	if (tp->regfunc && !static_key_enabled(&tp->key)) {
> > +	if (tp->regfunc &&
> > +	    ((tp->dynamic && !(atomic_read(&tp->key.enabled) > 0)) ||
> > +	     !static_key_enabled(&tp->key))) {
> > 		ret = tp->regfunc();
> > 		if (ret < 0)
> > 			return ret;
> > @@ -219,7 +221,9 @@ static int tracepoint_add_func(struct tracepoint *tp,
> > 	 * is used.
> > 	 */
> > 	rcu_assign_pointer(tp->funcs, tp_funcs);
> > -	if (!static_key_enabled(&tp->key))
> > +	if (tp->dynamic && !(atomic_read(&tp->key.enabled) > 0))
> > +		atomic_inc(&tp->key.enabled);
> > +	else if (!tp->dynamic && !static_key_enabled(&tp->key))
> > 		static_key_slow_inc(&tp->key);
> > 	release_probes(old);
> > 	return 0;
> > @@ -246,10 +250,14 @@ static int tracepoint_remove_func(struct tracepoint *tp,
> > 
> > 	if (!tp_funcs) {
> > 		/* Removed last function */
> > -		if (tp->unregfunc && static_key_enabled(&tp->key))
> > +		if (tp->unregfunc &&
> > +		    ((tp->dynamic && (atomic_read(&tp->key.enabled) > 0)) ||
> > +		     static_key_enabled(&tp->key)))
> > 			tp->unregfunc();
> > 
> > -		if (static_key_enabled(&tp->key))
> > +		if (tp->dynamic && (atomic_read(&tp->key.enabled) > 0))
> > +			atomic_dec(&tp->key.enabled);
> > +		else if (!tp->dynamic && static_key_enabled(&tp->key))
> > 			static_key_slow_dec(&tp->key);
> > 	}
> > 	rcu_assign_pointer(tp->funcs, tp_funcs);
> > @@ -258,7 +266,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
> > }
> > 
> > /**
> > - * tracepoint_probe_register -  Connect a probe to a tracepoint
> > + * tracepoint_probe_register_prio -  Connect a probe to a tracepoint
> >  * @tp: tracepoint
> >  * @probe: probe handler
> >  * @data: tracepoint data
> 
> Hi Tom,
> 
> Thanks for updating your approach to dynamic tracepoints.
> 
> Since you're fixing up this comment above tracepoint_probe_register_prio,
> can you also remove the following line above tracepoint_probe_register
> while you are at it ?
> 

Sure, will do.

Thanks,

Tom

[toc] | [prev] | [next] | [standalone]

#1728471

On Tue,  5 Sep 2017 16:57:37 -0500
Tom Zanussi <tom.zanussi@linux.intel.com> wrote:

> The tracepoint infrastructure assumes statically-defined tracepoints
> and uses static_keys for tracepoint enablement.  In order to define
> tracepoints on the fly, we need to have a dynamic counterpart.

Do we?

I believe the static keys should work just fine if you don't have any
defined static keys jumps, and they should work as a simple counter.

Could we do that instead of adding another variable to a structure that
is defined over a thousand times? Save us 4k of memory.

-- Steve

> 
> Add a 'dynamic' flag to struct tracepoint along with accompanying
> logic for this purpose.
> 
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> ---
>  include/linux/tracepoint-defs.h |  1 +
>  kernel/tracepoint.c             | 18 +++++++++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> index a031920..bc22d54 100644
> --- a/include/linux/tracepoint-defs.h
> +++ b/include/linux/tracepoint-defs.h
> @@ -32,6 +32,7 @@ struct tracepoint {
>  	int (*regfunc)(void);
>  	void (*unregfunc)(void);
>  	struct tracepoint_func __rcu *funcs;
> +	bool dynamic;
>  };
>  
>  #endif
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 685c50a..1c5957f 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -197,7 +197,9 @@ static int tracepoint_add_func(struct tracepoint *tp,
>  	struct tracepoint_func *old, *tp_funcs;
>  	int ret;
>  
> -	if (tp->regfunc && !static_key_enabled(&tp->key)) {
> +	if (tp->regfunc &&
> +	    ((tp->dynamic && !(atomic_read(&tp->key.enabled) > 0)) ||
> +	     !static_key_enabled(&tp->key))) {
>  		ret = tp->regfunc();
>  		if (ret < 0)
>  			return ret;
> @@ -219,7 +221,9 @@ static int tracepoint_add_func(struct tracepoint *tp,
>  	 * is used.
>  	 */
>  	rcu_assign_pointer(tp->funcs, tp_funcs);
> -	if (!static_key_enabled(&tp->key))
> +	if (tp->dynamic && !(atomic_read(&tp->key.enabled) > 0))
> +		atomic_inc(&tp->key.enabled);
> +	else if (!tp->dynamic && !static_key_enabled(&tp->key))
>  		static_key_slow_inc(&tp->key);
>  	release_probes(old);
>  	return 0;
> @@ -246,10 +250,14 @@ static int tracepoint_remove_func(struct tracepoint *tp,
>  
>  	if (!tp_funcs) {
>  		/* Removed last function */
> -		if (tp->unregfunc && static_key_enabled(&tp->key))
> +		if (tp->unregfunc &&
> +		    ((tp->dynamic && (atomic_read(&tp->key.enabled) > 0)) ||
> +		     static_key_enabled(&tp->key)))
>  			tp->unregfunc();
>  
> -		if (static_key_enabled(&tp->key))
> +		if (tp->dynamic && (atomic_read(&tp->key.enabled) > 0))
> +			atomic_dec(&tp->key.enabled);
> +		else if (!tp->dynamic && static_key_enabled(&tp->key))
>  			static_key_slow_dec(&tp->key);
>  	}
>  	rcu_assign_pointer(tp->funcs, tp_funcs);
> @@ -258,7 +266,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
>  }
>  
>  /**
> - * tracepoint_probe_register -  Connect a probe to a tracepoint
> + * tracepoint_probe_register_prio -  Connect a probe to a tracepoint
>   * @tp: tracepoint
>   * @probe: probe handler
>   * @data: tracepoint data

[toc] | [prev] | [next] | [standalone]

#1728970

Hi Steve,

On Thu, 2017-09-07 at 18:02 -0400, Steven Rostedt wrote:
> On Tue,  5 Sep 2017 16:57:37 -0500
> Tom Zanussi <tom.zanussi@linux.intel.com> wrote:
> 
> > The tracepoint infrastructure assumes statically-defined tracepoints
> > and uses static_keys for tracepoint enablement.  In order to define
> > tracepoints on the fly, we need to have a dynamic counterpart.
> 
> Do we?
> 
> I believe the static keys should work just fine if you don't have any
> defined static keys jumps, and they should work as a simple counter.
> 
> Could we do that instead of adding another variable to a structure that
> is defined over a thousand times? Save us 4k of memory.
> 

OK, yeah, makes sense if possible - let me look into that... 

Tom

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web