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 <m...@ellerman.id.au> > Reviewed-by: Madhavan Srinivasan <ma...@linux.vnet.ibm.com>
Applied to powerpc next. https://git.kernel.org/powerpc/c/db6711b7a17f03921e734e11e3a1e9 cheers
