linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Michael Ellerman <patch-notifications@ellerman.id.au>
To: Michael Ellerman <mpe@ellerman.id.au>, linuxppc-dev@ozlabs.org
Cc: maddy@linux.ibm.com, paulus@samba.org
Subject: Re: powerpc/perf: Add missing break in power7_marked_instr_event()
Date: Thu,  4 Oct 2018 16:14:37 +1000 (AEST)	[thread overview]
Message-ID: <42QjKB0TGvz9sj8@ozlabs.org> (raw)
In-Reply-To: <20180920094111.4125-1-mpe@ellerman.id.au>

On Thu, 2018-09-20 at 09:41:11 UTC, Michael Ellerman wrote:
> In power7_marked_instr_event() there is a switch case that is missing
> a break or an explicit fallthrough, it's not immediately clear which
> it should be.
> 
> The function determines based on the PMU event code, whether the event
> is a "marked" event (which then requires us to configure the PMU in a
> certain way). On Power7 there is no specific bit(s) in the event to
> tell us that, we just have to know.
> 
> Rather than having a full list of every event and whether they are
> marked, we pull apart the event code and for events with certain
> values of certain fields we can say that those are all marked events.
> 
> We take the psel (bits 0-7) of the event, and look at bits 4-7. For a
> value of 6 we say that if the entire psel == 0x64 then if the pmc == 3
> the event is marked, else not, and otherwise we continue.
> 
> It is then that we fallthrough to the 8 case, where we return true if
> the unit == 0xd.
> 
> The question is should the 6 case also fallthrough and check for
> unit == 0xd, or should it return.
> 
> Looking at the full list of events we see that there are zero events
> where (psel >> 4) == 0x6 and unit == 0xd.
> 
> So the answer is it doesn't really matter, there are no valid event
> codes that will return a different result whether we fallthrough or
> break.
> 
> But equally, testing the 6 case events against unit == 0xd is slightly
> bogus, as there are no such events. So to make the code clearer, and
> avoid any future confusion, have the 6 case break rather than falling
> through.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>

Applied to powerpc next.

https://git.kernel.org/powerpc/c/db6711b7a17f03921e734e11e3a1e9

cheers

      parent reply	other threads:[~2018-10-04  6:32 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-20  9:41 [PATCH] powerpc/perf: Add missing break in power7_marked_instr_event() Michael Ellerman
2018-09-20  9:59 ` Madhavan Srinivasan
2018-09-20 14:47   ` Segher Boessenkool
2018-09-24  7:03     ` Michael Ellerman
2018-09-24  7:05     ` Madhavan Srinivasan
2018-10-04  6:14 ` Michael Ellerman [this message]

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=42QjKB0TGvz9sj8@ozlabs.org \
    --to=patch-notifications@ellerman.id.au \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.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).