Re: [PATCH] perf/x86/intel: Mark expected switch fall-throughs

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Jun 26, 2019 at 11:24:32AM +0200, Peter Zijlstra wrote:
> On Tue, Jun 25, 2019 at 11:47:06PM +0200, Thomas Gleixner wrote:
> > > On Tue, Jun 25, 2019 at 09:53:09PM +0200, Thomas Gleixner wrote:
> 
> > > > but it also makes objtool unhappy:
> > > > 
> > > >  arch/x86/events/intel/core.o: warning: objtool: intel_pmu_nhm_workaround()+0xb3: unreachable instruction
> > > >  kernel/fork.o: warning: objtool: free_thread_stack()+0x126: unreachable instruction
> > > >  mm/workingset.o: warning: objtool: count_shadow_nodes()+0x11f: unreachable instruction
> > > >  arch/x86/kernel/cpu/mtrr/generic.o: warning: objtool: get_fixed_ranges()+0x9b: unreachable instruction
> > > >  arch/x86/kernel/platform-quirks.o: warning: objtool: x86_early_init_platform_quirks()+0x84: unreachable instruction
> > > >  drivers/iommu/irq_remapping.o: warning: objtool: irq_remap_enable_fault_handling()+0x1d: unreachable instruction
> 
> > I just checked two of them in the disassembly. In both cases it's jump
> > label related. Here is one:
> > 
> >       asm volatile("1: rdmsr\n"
> >  410:   b9 59 02 00 00          mov    $0x259,%ecx
> >  415:   0f 32                   rdmsr
> >  417:   49 89 c6                mov    %rax,%r14
> >  41a:   48 89 d3                mov    %rdx,%rbx
> >       return EAX_EDX_VAL(val, low, high);
> >  41d:   48 c1 e3 20             shl    $0x20,%rbx
> >  421:   48 09 c3                or     %rax,%rbx
> >  424:   0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
> >  429:   eb 0f                   jmp    43a <get_fixed_ranges+0xaa>
> >       do_trace_read_msr(msr, val, 0);
> >  42b:   bf 59 02 00 00          mov    $0x259,%edi   <------- "unreachable"
> >  430:   48 89 de                mov    %rbx,%rsi
> >  433:   31 d2                   xor    %edx,%edx
> >  435:   e8 00 00 00 00          callq  43a <get_fixed_ranges+0xaa>
> >  43a:   44 89 35 00 00 00 00    mov    %r14d,0x0(%rip)        # 441 <get_fixed_ranges+0xb1>
> > 
> > Interestingly enough there are some more hunks of the same pattern in that
> > function which look all the same. Those are not upsetting objtool. Josh
> > might give an hint where to stare at.
> 
> That's pretty atrocious code-gen :/ 

And I know nobody reads comments (I don't either), but I did write one
on this as it happens.

See include/linux/jump_label.h:399

/*
 * Combine the right initial value (type) with the right branch order
 * to generate the desired result.
 *
 *
 * type\branch|	likely (1)	      |	unlikely (0)
 * -----------+-----------------------+------------------
 *            |                       |
 *  true (1)  |	   ...		      |	   ...
 *            |    NOP		      |	   JMP L
 *            |    <br-stmts>	      |	1: ...
 *            |	L: ...		      |
 *            |			      |
 *            |			      |	L: <br-stmts>
 *            |			      |	   jmp 1b
 *            |                       |
 * -----------+-----------------------+------------------
 *            |                       |
 *  false (0) |	   ...		      |	   ...
 *            |    JMP L	      |	   NOP
 *            |    <br-stmts>	      |	1: ...
 *            |	L: ...		      |
 *            |			      |
 *            |			      |	L: <br-stmts>
 *            |			      |	   jmp 1b
 *            |                       |
 * -----------+-----------------------+------------------
 *
 * The initial value is encoded in the LSB of static_key::entries,
 * type: 0 = false, 1 = true.
 *
 * The branch type is encoded in the LSB of jump_entry::key,
 * branch: 0 = unlikely, 1 = likely.
 *
 * This gives the following logic table:
 *
 *	enabled	type	branch	  instuction
 * -----------------------------+-----------
 *	0	0	0	| NOP
 *	0	0	1	| JMP
 *	0	1	0	| NOP
 *	0	1	1	| JMP
 *
 *	1	0	0	| JMP
 *	1	0	1	| NOP
 *	1	1	0	| JMP
 *	1	1	1	| NOP
 *
 * Which gives the following functions:
 *
 *   dynamic: instruction = enabled ^ branch
 *   static:  instruction = type ^ branch
 *
 * See jump_label_type() / jump_label_init_type().
 */

The case here is false-unlikely, which *should* then result in the
branch statements going out-of-line.