Subject: Re: powerpc/perf: Add missing break in power7_marked_instr_event()

From: Michael Ellerman <[email-masked]>
Date: Thu, 4 Oct 2018 16:14:37 +1000 (AEST)

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 <[email-masked]>
> Reviewed-by: Madhavan Srinivasan <[email-masked]>

Applied to powerpc next.

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

cheers