trace name copy/paste in native_write_msr ?

trace name copy/paste in native_write_msr ?

[Dr. David Alan Gilbert]

Hi Andi,
  In arch/x86/include/asm/msr.h native_write_msr(_safe)
you added a trace test in 7f47d8cc, should the
tracepoint_active's be testing for __tracepoint_write_msr ?

/* Can be uninlined because referenced by paravirt */
notrace static inline void native_write_msr(unsigned int msr,
                                            unsigned low, unsigned high)
{
        asm volatile("1: wrmsr\n"
                     "2:\n"
                     _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
                     : : "c" (msr), "a"(low), "d" (high) : "memory");
        if (msr_tracepoint_active(__tracepoint_read_msr))   <---------------
                do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
}

/* Can be uninlined because referenced by paravirt */
notrace static inline int native_write_msr_safe(unsigned int msr,
                                        unsigned low, unsigned high)
{
        int err; 
        asm volatile("2: wrmsr ; xor %[err],%[err]\n"
                     "1:\n\t"
                     ".section .fixup,\"ax\"\n\t"
                     "3:  mov %[fault],%[err] ; jmp 1b\n\t"
                     ".previous\n\t"
                     _ASM_EXTABLE(2b, 3b)
                     : [err] "=a" (err)
                     : "c" (msr), "0" (low), "d" (high),
                       [fault] "i" (-EIO)
                     : "memory");
        if (msr_tracepoint_active(__tracepoint_read_msr))   <---------------
                do_trace_write_msr(msr, ((u64)high << 32 | low), err);
        return err;
}

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: trace name copy/paste in native_write_msr ?

[Andi Kleen]

On Fri, Jun 03, 2016 at 11:32:51AM +0100, Dr. David Alan Gilbert wrote:
> Hi Andi,
>   In arch/x86/include/asm/msr.h native_write_msr(_safe)
> you added a trace test in 7f47d8cc, should the
> tracepoint_active's be testing for __tracepoint_write_msr ?

Yes it should. Thanks for noticing. Please send a patch.

-Andi