On March 7, 2019 9:18:29 AM PST, Josh Poimboeuf <jpoim...@redhat.com> wrote:
>On Thu, Mar 07, 2019 at 09:04:36AM -0800, h...@zytor.com wrote:
>> On March 7, 2019 8:47:05 AM PST, Josh Poimboeuf <jpoim...@redhat.com>
>wrote:
>> >On Thu, Mar 07, 2019 at 02:13:12PM +0100, Peter Zijlstra wrote:
>> >> On Thu, Mar 07, 2019 at 01:55:26PM +0100, Peter Zijlstra wrote:
>> >> > On Thu, Mar 07, 2019 at 01:03:17PM +0100, Peter Zijlstra wrote:
>> >> 
>> >> 
>> >> > > 01be     20d3:  31 c0                   xor    %eax,%eax
>> >> > > 01c0     20d5:  4c 39 eb                cmp    %r13,%rbx
>> >> > > 01c3     20d8:  77 08                   ja     20e2
>> ><__do_sys_waitid+0x1cd>
>> >> 
>> >> randconfig-build/kernel/exit.o: warning: objtool:  
>> >__do_sys_waitid()+0x1c3: (branch)
>> >> 
>> >> > > 01cd     20e2:        83 f0 01                xor    $0x1,%eax
>> >> > > 01d0     20e5:        48 89 c2                mov    %rax,%rdx
>> >> > > 01d3     20e8:        83 e2 01                and    $0x1,%edx
>> >> > > 01d6     20eb:        48 83 c2 02             add    $0x2,%rdx
>> >> > > 01da     20ef:        48 ff 04 d5 00 00 00    incq   0x0(,%rdx,8)
>> >> > > 01e1     20f6:        00 
>> >> > > 01de                  20f3: R_X86_64_32S      _ftrace_branch+0x148
>> >> > > 01e2     20f7:        84 c0                   test   %al,%al
>> >> > > 01e4     20f9:        75 2d                   jne    2128
>> ><__do_sys_waitid+0x213>
>> >> > 
>> >> > we do not take this branch and fall-through.
>> >> 
>> >> And that is the error, I think. We should've taken it and went to:
>> >> 
>> >>   return -EFAULT;
>> >> 
>> >> because:
>> >> 
>> >>  +1be     xor  %eax,%eax  eax=0
>> >>  +1cd   xor  $0x1,%eax    eax=1
>> >>  +1e2   test %al,%al      1&1==1 -> ZF=0
>> >>  +1e4   jnz
>> >> 
>> >> Is an unconditional code sequence, but there's no way objtool can
>do
>> >> that without becoming a full blown interpreter :/
>> >> 
>> >> > > 0213     2128:  49 c7 c7 f2 ff ff ff    mov   
>> >$0xfffffffffffffff2,%r15
>> >> > > ffffffffffffe0eb }
>> >> > > 021a     212f:  48 8d 65 d8             lea   
>-0x28(%rbp),%rsp
>> >> > > 021e     2133:  4c 89 f8                mov    %r15,%rax
>> >> > > 0221     2136:  5b                      pop    %rbx
>> >> > > 0222     2137:  41 5c                   pop    %r12
>> >> > > 0224     2139:  41 5d                   pop    %r13
>> >> > > 0226     213b:  41 5e                   pop    %r14
>> >> > > 0228     213d:  41 5f                   pop    %r15
>> >> > > 022a     213f:  5d                      pop    %rbp
>> >> > > 022b     2140:  c3                      retq
>> >
>> >This "fixes" it, and also seems to help -Os make much code:
>> >
>> >diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> >index 445348facea9..8de63db58fdd 100644
>> >--- a/include/linux/compiler.h
>> >+++ b/include/linux/compiler.h
>> >@@ -67,7 +67,7 @@ void ftrace_likely_update(struct
>ftrace_likely_data
>> >*f, int val,
>> >                            .line = __LINE__,                       \
>> >                    };                                              \
>> >            ______r = !!(cond);                                     \
>> >-           ______f.miss_hit[______r]++;                                    
>> >\
>> >+           if (______r) ______f.miss_hit[1]++; else ______f.miss_hit[0]++; 
>> >\
>> >            ______r;                                                \
>> >    }))
>> > #endif /* CONFIG_PROFILE_ALL_BRANCHES */
>> 
>> if (cond)?  Or is ___r used elsewhere?
>
>______r is also the return value.  And it's needed because cond should
>only be evaluated once.

So put a true; and false; inside the if.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Reply via email to