[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190726144310.5c62925b@gandalf.local.home>
Date: Fri, 26 Jul 2019 14:43:10 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Matt Helsley <mhelsley@...are.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v3 04/13] recordmcount: Rewrite error/success handling
On Fri, 26 Jul 2019 18:37:11 +0000
Matt Helsley <mhelsley@...are.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
Powered by blists - more mailing lists