linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Matt Helsley <mhelsley@vmware.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH v3 04/13] recordmcount: Rewrite error/success handling
Date: Fri, 26 Jul 2019 14:43:10 -0400	[thread overview]
Message-ID: <20190726144310.5c62925b@gandalf.local.home> (raw)
In-Reply-To: <4F0912A7-345E-46EE-B58F-CF7E8EE2AB65@vmware.com>

On Fri, 26 Jul 2019 18:37:11 +0000
Matt Helsley <mhelsley@vmware.com> wrote:
 
> >> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> >> index c1e1b04b4871..909a3e4775c2 100644
> >> --- a/scripts/recordmcount.h
> >> +++ b/scripts/recordmcount.h
> >> @@ -24,7 +24,9 @@
> >> #undef mcount_adjust
> >> #undef sift_rel_mcount
> >> #undef nop_mcount
> >> +#undef missing_sym
> >> #undef find_secsym_ndx
> >> +#undef already_has_rel_mcount  
> > 
> > Why do we need these as defines? Can't you just have a single:
> > 
> > const char *already_has_mcount = "success";
> > 
> > in recordmcount.c before recordmcount.h is included?
> > 
> > And same for missing_sym.  
> 
> Yes, that’s a good point. I’ve been trying to separate the changes to
> the functions from moving parts out but in this case it would make
> just as much sense to add them to recordmcount.c in the first place.
> 
> Ultimately, this ugliness gets removed as the next series removes
> recordmcount.h entirely and one of the steps is moving
> find_secsym_ndx() out while eliminating these redundant pieces.

Yeah, this code will be cleaned up later, but let's have the steps in
between look fine as well.


> 
> > 
> > Another, probably more robust way of doing this, is change
> > find_secsym_ndx() to return 0 on success and -1 on missing symbol,
> > and just pass a pointer by reference to fill the recsym (which
> > doesn't have to be a constant).  
> 
> That’s easy enough to do  and  I do like separating the error/success
> return from returning  the index. I can send that out now or tack it
> onto the next RFC series I’m about to send which completes the
> conversion if that’s preferable.
> 
> Yeah, the original code applies “const” in lots of places -- I
> presume it’s an attempt to eek out every last bit of performance from
> the compiler.

As I said before, I've applied patches 1-3, so you don't need to resend
them. I finished looking at the rest, and only this patch needs to be
fixed, and since you are resending, could you fix the "upside-down
x-mas" tree declaration I mentioned in patch 8.

Thanks Matt,

-- Steve

  reply	other threads:[~2019-07-26 18:43 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-24 21:04 [PATCH v3 00/13] Cleanup recordmcount and begin objtool conversion Matt Helsley
2019-07-24 21:04 ` [PATCH v3 01/13] recordmcount: Remove redundant strcmp Matt Helsley
2019-07-24 21:04 ` [PATCH v3 02/13] recordmcount: Remove uread() Matt Helsley
2019-07-24 21:04 ` [PATCH v3 03/13] recordmcount: Remove unused fd from uwrite() and ulseek() Matt Helsley
2019-07-24 21:04 ` [PATCH v3 04/13] recordmcount: Rewrite error/success handling Matt Helsley
2019-07-26 17:45   ` Steven Rostedt
2019-07-26 18:37     ` Matt Helsley
2019-07-26 18:43       ` Steven Rostedt [this message]
2019-07-26 19:23         ` Matt Helsley
2019-07-24 21:04 ` [PATCH v3 05/13] recordmcount: Kernel style function signature formatting Matt Helsley
2019-07-24 21:05 ` [PATCH v3 06/13] recordmcount: Kernel style formatting Matt Helsley
2019-07-24 21:05 ` [PATCH v3 07/13] recordmcount: Remove redundant cleanup() calls Matt Helsley
2019-07-24 21:05 ` [PATCH v3 08/13] recordmcount: Clarify what cleanup() does Matt Helsley
2019-07-26 16:10   ` Steven Rostedt
2019-07-26 16:13   ` Steven Rostedt
2019-07-24 21:05 ` [PATCH v3 09/13] objtool: Prepare to merge recordmcount Matt Helsley
2019-07-28 17:48   ` Josh Poimboeuf
2019-07-29 20:10     ` Matt Helsley
2019-07-24 21:05 ` [PATCH v3 10/13] objtool: Make recordmcount into an objtool subcmd Matt Helsley
2019-07-28 17:48   ` Josh Poimboeuf
2019-07-29 20:19     ` Matt Helsley
2019-07-24 21:05 ` [PATCH v3 11/13] objtool: recordmcount: Start using objtool's elf wrapper Matt Helsley
2019-07-24 21:05 ` [PATCH v3 12/13] objtool: recordmcount: Search for __mcount_loc before walking the sections Matt Helsley
2019-07-24 21:05 ` [PATCH v3 13/13] objtool: recordmcount: Convert do_func() relhdrs Matt Helsley
2019-07-28 17:52 ` [PATCH v3 00/13] Cleanup recordmcount and begin objtool conversion Josh Poimboeuf

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=20190726144310.5c62925b@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhelsley@vmware.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).