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

Groups > linux.kernel > #1563267 > unrolled thread

[PATCH 1/3] ftrace: Factor out __ftrace_hash_move()

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/3] ftrace: Factor out __ftrace_hash_move() Namhyung Kim <namhyung@kernel.org> - 2017-01-20 04:30 +0100
    [PATCH 2/3] ftrace: Expose ftrace_hash_empty and ftrace_lookup_ip Namhyung Kim <namhyung@kernel.org> - 2017-01-20 04:30 +0100
      Re: [PATCH 2/3] ftrace: Expose ftrace_hash_empty and  ftrace_lookup_ip Steven Rostedt <rostedt@goodmis.org> - 2017-01-20 21:00 +0100
        Re: [PATCH 2/3] ftrace: Expose ftrace_hash_empty and ftrace_lookup_ip Namhyung Kim <namhyung@kernel.org> - 2017-01-21 01:30 +0100
          Re: [PATCH 2/3] ftrace: Expose ftrace_hash_empty and  ftrace_lookup_ip Steven Rostedt <rostedt@goodmis.org> - 2017-01-21 03:20 +0100
            Re: [PATCH 2/3] ftrace: Expose ftrace_hash_empty and ftrace_lookup_ip Namhyung Kim <namhyung@kernel.org> - 2017-01-21 03:30 +0100
    Re: [PATCH 1/3] ftrace: Factor out __ftrace_hash_move() Steven Rostedt <rostedt@goodmis.org> - 2017-01-20 18:30 +0100
      Re: [PATCH 1/3] ftrace: Factor out __ftrace_hash_move() Namhyung Kim <namhyung@kernel.org> - 2017-01-21 01:30 +0100

#1563267 — [PATCH 1/3] ftrace: Factor out __ftrace_hash_move()

The __ftrace_hash_move() is to allocates properly-sized hash and move
entries in the src ftrace_hash.  It will be used to set function graph
filters which has nothing to do with the dyn_ftrace records.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 kernel/trace/ftrace.c | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index eb230f06ba41..37b0e948d924 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1383,9 +1383,8 @@ ftrace_hash_rec_enable_modify(struct ftrace_ops *ops, int filter_hash);
 static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops,
 				       struct ftrace_hash *new_hash);
 
-static int
-ftrace_hash_move(struct ftrace_ops *ops, int enable,
-		 struct ftrace_hash **dst, struct ftrace_hash *src)
+static struct ftrace_hash *
+__ftrace_hash_move(struct ftrace_hash *src)
 {
 	struct ftrace_func_entry *entry;
 	struct hlist_node *tn;
@@ -1393,21 +1392,13 @@ ftrace_hash_move(struct ftrace_ops *ops, int enable,
 	struct ftrace_hash *new_hash;
 	int size = src->count;
 	int bits = 0;
-	int ret;
 	int i;
 
-	/* Reject setting notrace hash on IPMODIFY ftrace_ops */
-	if (ops->flags & FTRACE_OPS_FL_IPMODIFY && !enable)
-		return -EINVAL;
-
 	/*
-	 * If the new source is empty, just free dst and assign it
-	 * the empty_hash.
+	 * If the new source is empty, just return the empty_hash.
 	 */
-	if (!src->count) {
-		new_hash = EMPTY_HASH;
-		goto update;
-	}
+	if (!src->count)
+		return EMPTY_HASH;
 
 	/*
 	 * Make the hash size about 1/2 the # found
@@ -1421,7 +1412,7 @@ ftrace_hash_move(struct ftrace_ops *ops, int enable,
 
 	new_hash = alloc_ftrace_hash(bits);
 	if (!new_hash)
-		return -ENOMEM;
+		return NULL;
 
 	size = 1 << src->size_bits;
 	for (i = 0; i < size; i++) {
@@ -1432,7 +1423,24 @@ ftrace_hash_move(struct ftrace_ops *ops, int enable,
 		}
 	}
 
-update:
+	return new_hash;
+}
+
+static int
+ftrace_hash_move(struct ftrace_ops *ops, int enable,
+		 struct ftrace_hash **dst, struct ftrace_hash *src)
+{
+	struct ftrace_hash *new_hash;
+	int ret;
+
+	/* Reject setting notrace hash on IPMODIFY ftrace_ops */
+	if (ops->flags & FTRACE_OPS_FL_IPMODIFY && !enable)
+		return -EINVAL;
+
+	new_hash = __ftrace_hash_move(src);
+	if (!new_hash)
+		return -ENOMEM;
+
 	/* Make sure this can be applied if it is IPMODIFY ftrace_ops */
 	if (enable) {
 		/* IPMODIFY should be updated only when filter_hash updating */
-- 
2.11.0

[toc] | [next] | [standalone]

#1563268 — [PATCH 2/3] ftrace: Expose ftrace_hash_empty and ftrace_lookup_ip

It will be used when checking graph filter hashes later.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 kernel/trace/ftrace.c | 14 +-------------
 kernel/trace/trace.h  | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 37b0e948d924..0470e373b9b4 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1110,13 +1110,6 @@ struct ftrace_func_entry {
 	unsigned long ip;
 };
 
-struct ftrace_hash {
-	unsigned long		size_bits;
-	struct hlist_head	*buckets;
-	unsigned long		count;
-	struct rcu_head		rcu;
-};
-
 /*
  * We make these constant because no one should touch them,
  * but they are used as the default "empty hash", to avoid allocating
@@ -1192,12 +1185,7 @@ struct ftrace_page {
 static struct ftrace_page	*ftrace_pages_start;
 static struct ftrace_page	*ftrace_pages;
 
-static bool __always_inline ftrace_hash_empty(struct ftrace_hash *hash)
-{
-	return !hash || !hash->count;
-}
-
-static struct ftrace_func_entry *
+struct ftrace_func_entry *
 ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip)
 {
 	unsigned long key;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 1ea51ab53edf..431a39ba8eee 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -787,6 +787,20 @@ extern void __trace_graph_return(struct trace_array *tr,
 				 struct ftrace_graph_ret *trace,
 				 unsigned long flags, int pc);
 
+struct ftrace_hash {
+	unsigned long		size_bits;
+	struct hlist_head	*buckets;
+	unsigned long		count;
+	struct rcu_head		rcu;
+};
+
+struct ftrace_func_entry *
+ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip);
+
+static bool __always_inline ftrace_hash_empty(struct ftrace_hash *hash)
+{
+	return !hash || !hash->count;
+}
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 /* TODO: make this variable */
-- 
2.11.0

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

#1563886 — Re: [PATCH 2/3] ftrace: Expose ftrace_hash_empty and ftrace_lookup_ip

On Fri, 20 Jan 2017 11:44:46 +0900
Namhyung Kim <namhyung@kernel.org> wrote:


> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -787,6 +787,20 @@ extern void __trace_graph_return(struct trace_array *tr,
>  				 struct ftrace_graph_ret *trace,
>  				 unsigned long flags, int pc);
>  
> +struct ftrace_hash {
> +	unsigned long		size_bits;
> +	struct hlist_head	*buckets;
> +	unsigned long		count;
> +	struct rcu_head		rcu;
> +};
> +
> +struct ftrace_func_entry *
> +ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip);
> +
> +static bool __always_inline ftrace_hash_empty(struct ftrace_hash *hash)
> +{
> +	return !hash || !hash->count;
> +}

Note, I had to modify this patch and move this declaration outside of
the #ifdef CONFIG_FUNCTION_GRAPH_TRACER, as it failed to build when
function graph wasn't enabled. Function tracer uses this too.

-- Steve

>  
>  #ifdef CONFIG_DYNAMIC_FTRACE
>  /* TODO: make this variable */

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

#1564011 — Re: [PATCH 2/3] ftrace: Expose ftrace_hash_empty and ftrace_lookup_ip

On Fri, Jan 20, 2017 at 02:54:20PM -0500, Steven Rostedt wrote:
> On Fri, 20 Jan 2017 11:44:46 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> 
> > --- a/kernel/trace/trace.h
> > +++ b/kernel/trace/trace.h
> > @@ -787,6 +787,20 @@ extern void __trace_graph_return(struct trace_array *tr,
> >  				 struct ftrace_graph_ret *trace,
> >  				 unsigned long flags, int pc);
> >  
> > +struct ftrace_hash {
> > +	unsigned long		size_bits;
> > +	struct hlist_head	*buckets;
> > +	unsigned long		count;
> > +	struct rcu_head		rcu;
> > +};
> > +
> > +struct ftrace_func_entry *
> > +ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip);
> > +
> > +static bool __always_inline ftrace_hash_empty(struct ftrace_hash *hash)
> > +{
> > +	return !hash || !hash->count;
> > +}
> 
> Note, I had to modify this patch and move this declaration outside of
> the #ifdef CONFIG_FUNCTION_GRAPH_TRACER, as it failed to build when
> function graph wasn't enabled. Function tracer uses this too.

Oops, my bad.

Did you modify it in your tree?  Or do you want me to resend v2?

Thanks,
Namhyung


> >  
> >  #ifdef CONFIG_DYNAMIC_FTRACE
> >  /* TODO: make this variable */
> 

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

#1564040 — Re: [PATCH 2/3] ftrace: Expose ftrace_hash_empty and ftrace_lookup_ip

On Sat, 21 Jan 2017 09:27:06 +0900
Namhyung Kim <namhyung@kernel.org> wrote:


> > Note, I had to modify this patch and move this declaration outside of
> > the #ifdef CONFIG_FUNCTION_GRAPH_TRACER, as it failed to build when
> > function graph wasn't enabled. Function tracer uses this too.  
> 
> Oops, my bad.
> 
> Did you modify it in your tree?  Or do you want me to resend v2?

No, it's a trivial change and I made the modification and noted it in
the change log. You can see the change in my tree under the ftrace/core
branch. I'm taking off next week and I wanted to get this series tested
before I go. The tests are still running.

-- Steve

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

#1564044 — Re: [PATCH 2/3] ftrace: Expose ftrace_hash_empty and ftrace_lookup_ip

On Fri, Jan 20, 2017 at 09:13:17PM -0500, Steven Rostedt wrote:
> On Sat, 21 Jan 2017 09:27:06 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> 
> > > Note, I had to modify this patch and move this declaration outside of
> > > the #ifdef CONFIG_FUNCTION_GRAPH_TRACER, as it failed to build when
> > > function graph wasn't enabled. Function tracer uses this too.  
> > 
> > Oops, my bad.
> > 
> > Did you modify it in your tree?  Or do you want me to resend v2?
> 
> No, it's a trivial change and I made the modification and noted it in
> the change log. You can see the change in my tree under the ftrace/core
> branch. I'm taking off next week and I wanted to get this series tested
> before I go. The tests are still running.

Thanks for doing that!
Namhyung

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

#1563808

On Fri, 20 Jan 2017 11:44:45 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> The __ftrace_hash_move() is to allocates properly-sized hash and move
> entries in the src ftrace_hash.  It will be used to set function graph
> filters which has nothing to do with the dyn_ftrace records.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Hmm, no cover letter?

Anyway, I applied the patches and I'm testing them now. I took a look
over them and they seem good, although, there's a few optimizations I
want to add. But I'll do that later.

Thanks!

-- Steve

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

#1564014

Hi Steve,

On Fri, Jan 20, 2017 at 12:25:35PM -0500, Steven Rostedt wrote:
> On Fri, 20 Jan 2017 11:44:45 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> > The __ftrace_hash_move() is to allocates properly-sized hash and move
> > entries in the src ftrace_hash.  It will be used to set function graph
> > filters which has nothing to do with the dyn_ftrace records.
> > 
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Hmm, no cover letter?

Oh, I thought it's a conceptually simple and small patch series.  So I
didn't add a cover letter.  Do you prefer seeing a cover letter anyway?

> 
> Anyway, I applied the patches and I'm testing them now. I took a look
> over them and they seem good, although, there's a few optimizations I
> want to add. But I'll do that later.

Thanks, if you have anything for me to do, please let me know.
Namhyung

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web