All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arch@vger.kernel.org,
	Joel Fernandes <joel@joelfernandes.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Andy Lutomirski <luto@kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	stable@kernel.org
Subject: Re: [for-next][PATCH 15/18] function_graph: Use new curr_ret_depth to manage depth instead of curr_ret_stack
Date: Fri, 23 Nov 2018 09:14:47 -0500	[thread overview]
Message-ID: <20181123091447.0e2eed1b@vmware.local.home> (raw)
In-Reply-To: <20181122100322.GN2131@hirez.programming.kicks-ass.net>

On Thu, 22 Nov 2018 11:03:22 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Nov 21, 2018 at 07:28:16PM -0500, Steven Rostedt wrote:
> > @@ -188,14 +186,20 @@ int function_graph_enter(unsigned long ret, unsigned long func,
> >  	struct ftrace_graph_ent trace;
> >  
> >  	trace.func = func;
> > -	trace.depth = current->curr_ret_stack + 1;
> > +	trace.depth = ++current->curr_ret_depth;
> >  
> >  	/* Only trace if the calling function expects to */
> >  	if (!ftrace_graph_entry(&trace))
> > -		return -EBUSY;
> > +		goto out;
> >  
> > -	return ftrace_push_return_trace(ret, func, &trace.depth,
> > -					frame_pointer, retp);
> > +	if (ftrace_push_return_trace(ret, func,
> > +				     frame_pointer, retp))  
> 
> You can unwrap that line, by my counting that's at 69 chars, so unless
> you're editing on a C64 it should fit your screen.

Thanks I may add a clean up patch. I already ran this code under
several hours of testing, and only plan on touching it if there's more
than cosmetic issues.

The reason that looks like that is because I just massaged the line it
replaced, and didn't take into account that I could now make it into a
single line.

> 
> > +		goto out;
> > +
> > +	return 0;
> > + out:
> > +	current->curr_ret_depth--;
> > +	return -EBUSY;
> >  }  
> 
> [diff "default"]
> 	xfuncname = "^[[:alpha:]$_].*[^:]$"
> 
> avoids the need for that ludicrous label indenting.

I still prefer the 'space', and it fits with the rest of the file ;-)

TomAYto / TomAHto

> 
> Also, "error" might be a better label name.

True. I'll add that on top too, but for the stable/rc release, that
isn't needed.

Thanks for taking a look at this code.

-- Steve

  reply	other threads:[~2018-11-23 14:14 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-22  0:28 [for-next][PATCH 00/18] function_graph: Add separate depth counter to prevent trace corruption Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 01/18] function_graph: Create function_graph_enter() to consolidate architecture code Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 02/18] x86/function_graph: Simplify with function_graph_entry() Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 03/18] ARM: function_graph: " Steven Rostedt
2018-11-22  0:28   ` Steven Rostedt
2018-11-23  7:30   ` Sasha Levin
2018-11-22  0:28 ` [for-next][PATCH 04/18] arm64: " Steven Rostedt
2018-11-22  0:28   ` Steven Rostedt
2018-11-22  0:28   ` Steven Rostedt
2018-11-27 18:07   ` Will Deacon
2018-11-27 18:07     ` Will Deacon
2018-11-27 18:26     ` Steven Rostedt
2018-11-27 18:26       ` Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 05/18] microblaze: " Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 06/18] MIPS: " Steven Rostedt
2018-11-23  7:30   ` Sasha Levin
2018-11-23  7:30     ` Sasha Levin
2018-11-22  0:28 ` [for-next][PATCH 07/18] nds32: " Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 08/18] parisc: " Steven Rostedt
     [not found]   ` <20181123073033.2083020863@mail.kernel.org>
2018-11-23  9:06     ` Helge Deller
2018-11-23 17:12       ` Steven Rostedt
2018-11-23 18:34         ` Sasha Levin
2018-11-23 19:26           ` Steven Rostedt
2018-11-23 20:00             ` Sasha Levin
2018-11-24 18:46               ` Steven Rostedt
2018-12-03 14:54                 ` Sasha Levin
2018-11-22  0:28 ` [for-next][PATCH 09/18] powerpc/function_graph: " Steven Rostedt
2018-11-22  0:28   ` Steven Rostedt
2018-11-23  7:30   ` Sasha Levin
2018-11-22  0:28 ` [for-next][PATCH 10/18] riscv/function_graph: " Steven Rostedt
2018-11-26 16:47   ` Palmer Dabbelt
2018-11-22  0:28 ` [for-next][PATCH 11/18] s390/function_graph: " Steven Rostedt
2018-11-22  6:43   ` Martin Schwidefsky
2018-11-23 17:15     ` Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 12/18] sh/function_graph: " Steven Rostedt
2018-11-22  0:28   ` Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 13/18] sparc/function_graph: " Steven Rostedt
2018-11-22  0:28   ` Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 14/18] function_graph: Make ftrace_push_return_trace() static Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 15/18] function_graph: Use new curr_ret_depth to manage depth instead of curr_ret_stack Steven Rostedt
2018-11-22 10:03   ` Peter Zijlstra
2018-11-23 14:14     ` Steven Rostedt [this message]
2018-11-22  0:28 ` [for-next][PATCH 16/18] function_graph: Move return callback before update " Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 17/18] function_graph: Reverse the order of pushing the ret_stack and the callback Steven Rostedt
2018-11-22  0:28 ` [for-next][PATCH 18/18] function_graph: Have profiler use curr_ret_stack and not depth Steven Rostedt
2018-11-26  5:15 ` [for-next][PATCH 00/18] function_graph: Add separate depth counter to prevent trace corruption Masami Hiramatsu
2018-11-28 20:39 ` Joe Lawrence
2018-11-28 21:00   ` Steven Rostedt
2018-11-29  3:29     ` Steven Rostedt
2018-11-29  4:24       ` Steven Rostedt
2018-11-29 15:08         ` Joe Lawrence
2018-11-29 16:17           ` Steven Rostedt
2018-11-29 16:32             ` Joe Lawrence

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181123091447.0e2eed1b@vmware.local.home \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=frederic@kernel.org \
    --cc=joel@joelfernandes.org \
    --cc=jpoimboe@redhat.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.