Re: [PATCH 3/3 v2] x86: Use tracepoint_enabled() for msr tracepoints instead of open coding it

From: kernel test robot <lkp@intel.com>
To: Steven Rostedt <rostedt@goodmis.org>, linux-kernel@vger.kernel.org
Cc: kbuild-all@lists.01.org, clang-built-linux@googlegroups.com,
	Yafang Shao <laoar.shao@gmail.com>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Memory Management List <linux-mm@kvack.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	Michel Lespinasse <walken@google.com>,
	Daniel Jordan <daniel.m.jordan@oracle.com>,
	Davidlohr Bueso <dbueso@suse.de>, Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH 3/3 v2] x86: Use tracepoint_enabled() for msr tracepoints instead of open coding it
Date: Sat, 26 Sep 2020 14:55:13 +0800	[thread overview]
Message-ID: <202009261429.HkE0bTfh%lkp@intel.com> (raw)
In-Reply-To: <20200925211820.050807067@goodmis.org>

[-- Attachment #1: Type: text/plain, Size: 10133 bytes --]

Hi Steven,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/master]
[also build test ERROR on linux/master tip/perf/core tip/x86/core linus/master v5.9-rc6 next-20200925]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Steven-Rostedt/tracing-mm-Add-tracepoint_enabled-helper-function-for-headers/20200926-051950
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 0248dedd12d43035bf53c326633f0610a49d7134
config: x86_64-randconfig-a003-20200925 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project a83eb048cb9a75da7a07a9d5318bbdbf54885c87)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/7a9f7773ebb9b2d4be989415cfa2cee5788201ea
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Steven-Rostedt/tracing-mm-Add-tracepoint_enabled-helper-function-for-headers/20200926-051950
        git checkout 7a9f7773ebb9b2d4be989415cfa2cee5788201ea
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/x86/kernel/asm-offsets.c:9:
   In file included from include/linux/crypto.h:20:
   In file included from include/linux/slab.h:15:
   In file included from include/linux/gfp.h:6:
   In file included from include/linux/mmzone.h:8:
   In file included from include/linux/spinlock.h:51:
   In file included from include/linux/preempt.h:78:
   In file included from arch/x86/include/asm/preempt.h:7:
   In file included from include/linux/thread_info.h:38:
   In file included from arch/x86/include/asm/thread_info.h:53:
   In file included from arch/x86/include/asm/cpufeature.h:5:
   In file included from arch/x86/include/asm/processor.h:22:
>> arch/x86/include/asm/msr.h:129:6: error: implicit declaration of function 'tracepoint_enabled' [-Werror,-Wimplicit-function-declaration]
           if (tracepoint_enabled(read_msr))
               ^
>> arch/x86/include/asm/msr.h:129:25: error: use of undeclared identifier 'read_msr'
           if (tracepoint_enabled(read_msr))
                                  ^
   arch/x86/include/asm/msr.h:151:6: error: implicit declaration of function 'tracepoint_enabled' [-Werror,-Wimplicit-function-declaration]
           if (tracepoint_enabled(read_msr))
               ^
   arch/x86/include/asm/msr.h:151:25: error: use of undeclared identifier 'read_msr'
           if (tracepoint_enabled(read_msr))
                                  ^
   arch/x86/include/asm/msr.h:162:6: error: implicit declaration of function 'tracepoint_enabled' [-Werror,-Wimplicit-function-declaration]
           if (tracepoint_enabled(write_msr))
               ^
>> arch/x86/include/asm/msr.h:162:25: error: use of undeclared identifier 'write_msr'
           if (tracepoint_enabled(write_msr))
                                  ^
   arch/x86/include/asm/msr.h:182:6: error: implicit declaration of function 'tracepoint_enabled' [-Werror,-Wimplicit-function-declaration]
           if (tracepoint_enabled(write_msr))
               ^
   arch/x86/include/asm/msr.h:182:25: error: use of undeclared identifier 'write_msr'
           if (tracepoint_enabled(write_msr))
                                  ^
   arch/x86/include/asm/msr.h:249:6: error: implicit declaration of function 'tracepoint_enabled' [-Werror,-Wimplicit-function-declaration]
           if (tracepoint_enabled(rdpmc))
               ^
>> arch/x86/include/asm/msr.h:249:25: error: use of undeclared identifier 'rdpmc'; did you mean 'rdtsc'?
           if (tracepoint_enabled(rdpmc))
                                  ^~~~~
                                  rdtsc
   arch/x86/include/asm/msr.h:199:43: note: 'rdtsc' declared here
   static __always_inline unsigned long long rdtsc(void)
                                             ^
   10 errors generated.
   make[2]: *** [scripts/Makefile.build:117: arch/x86/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1198: prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

vim +/tracepoint_enabled +129 arch/x86/include/asm/msr.h

   115	
   116	#define native_wrmsr(msr, low, high)			\
   117		__wrmsr(msr, low, high)
   118	
   119	#define native_wrmsrl(msr, val)				\
   120		__wrmsr((msr), (u32)((u64)(val)),		\
   121			       (u32)((u64)(val) >> 32))
   122	
   123	static inline unsigned long long native_read_msr(unsigned int msr)
   124	{
   125		unsigned long long val;
   126	
   127		val = __rdmsr(msr);
   128	
 > 129		if (tracepoint_enabled(read_msr))
   130			do_trace_read_msr(msr, val, 0);
   131	
   132		return val;
   133	}
   134	
   135	static inline unsigned long long native_read_msr_safe(unsigned int msr,
   136							      int *err)
   137	{
   138		DECLARE_ARGS(val, low, high);
   139	
   140		asm volatile("2: rdmsr ; xor %[err],%[err]\n"
   141			     "1:\n\t"
   142			     ".section .fixup,\"ax\"\n\t"
   143			     "3: mov %[fault],%[err]\n\t"
   144			     "xorl %%eax, %%eax\n\t"
   145			     "xorl %%edx, %%edx\n\t"
   146			     "jmp 1b\n\t"
   147			     ".previous\n\t"
   148			     _ASM_EXTABLE(2b, 3b)
   149			     : [err] "=r" (*err), EAX_EDX_RET(val, low, high)
   150			     : "c" (msr), [fault] "i" (-EIO));
   151		if (tracepoint_enabled(read_msr))
   152			do_trace_read_msr(msr, EAX_EDX_VAL(val, low, high), *err);
   153		return EAX_EDX_VAL(val, low, high);
   154	}
   155	
   156	/* Can be uninlined because referenced by paravirt */
   157	static inline void notrace
   158	native_write_msr(unsigned int msr, u32 low, u32 high)
   159	{
   160		__wrmsr(msr, low, high);
   161	
 > 162		if (tracepoint_enabled(write_msr))
   163			do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
   164	}
   165	
   166	/* Can be uninlined because referenced by paravirt */
   167	static inline int notrace
   168	native_write_msr_safe(unsigned int msr, u32 low, u32 high)
   169	{
   170		int err;
   171	
   172		asm volatile("2: wrmsr ; xor %[err],%[err]\n"
   173			     "1:\n\t"
   174			     ".section .fixup,\"ax\"\n\t"
   175			     "3:  mov %[fault],%[err] ; jmp 1b\n\t"
   176			     ".previous\n\t"
   177			     _ASM_EXTABLE(2b, 3b)
   178			     : [err] "=a" (err)
   179			     : "c" (msr), "0" (low), "d" (high),
   180			       [fault] "i" (-EIO)
   181			     : "memory");
   182		if (tracepoint_enabled(write_msr))
   183			do_trace_write_msr(msr, ((u64)high << 32 | low), err);
   184		return err;
   185	}
   186	
   187	extern int rdmsr_safe_regs(u32 regs[8]);
   188	extern int wrmsr_safe_regs(u32 regs[8]);
   189	
   190	/**
   191	 * rdtsc() - returns the current TSC without ordering constraints
   192	 *
   193	 * rdtsc() returns the result of RDTSC as a 64-bit integer.  The
   194	 * only ordering constraint it supplies is the ordering implied by
   195	 * "asm volatile": it will put the RDTSC in the place you expect.  The
   196	 * CPU can and will speculatively execute that RDTSC, though, so the
   197	 * results can be non-monotonic if compared on different CPUs.
   198	 */
   199	static __always_inline unsigned long long rdtsc(void)
   200	{
   201		DECLARE_ARGS(val, low, high);
   202	
   203		asm volatile("rdtsc" : EAX_EDX_RET(val, low, high));
   204	
   205		return EAX_EDX_VAL(val, low, high);
   206	}
   207	
   208	/**
   209	 * rdtsc_ordered() - read the current TSC in program order
   210	 *
   211	 * rdtsc_ordered() returns the result of RDTSC as a 64-bit integer.
   212	 * It is ordered like a load to a global in-memory counter.  It should
   213	 * be impossible to observe non-monotonic rdtsc_unordered() behavior
   214	 * across multiple CPUs as long as the TSC is synced.
   215	 */
   216	static __always_inline unsigned long long rdtsc_ordered(void)
   217	{
   218		DECLARE_ARGS(val, low, high);
   219	
   220		/*
   221		 * The RDTSC instruction is not ordered relative to memory
   222		 * access.  The Intel SDM and the AMD APM are both vague on this
   223		 * point, but empirically an RDTSC instruction can be
   224		 * speculatively executed before prior loads.  An RDTSC
   225		 * immediately after an appropriate barrier appears to be
   226		 * ordered as a normal load, that is, it provides the same
   227		 * ordering guarantees as reading from a global memory location
   228		 * that some other imaginary CPU is updating continuously with a
   229		 * time stamp.
   230		 *
   231		 * Thus, use the preferred barrier on the respective CPU, aiming for
   232		 * RDTSCP as the default.
   233		 */
   234		asm volatile(ALTERNATIVE_2("rdtsc",
   235					   "lfence; rdtsc", X86_FEATURE_LFENCE_RDTSC,
   236					   "rdtscp", X86_FEATURE_RDTSCP)
   237				: EAX_EDX_RET(val, low, high)
   238				/* RDTSCP clobbers ECX with MSR_TSC_AUX. */
   239				:: "ecx");
   240	
   241		return EAX_EDX_VAL(val, low, high);
   242	}
   243	
   244	static inline unsigned long long native_read_pmc(int counter)
   245	{
   246		DECLARE_ARGS(val, low, high);
   247	
   248		asm volatile("rdpmc" : EAX_EDX_RET(val, low, high) : "c" (counter));
 > 249		if (tracepoint_enabled(rdpmc))
   250			do_trace_rdpmc(counter, EAX_EDX_VAL(val, low, high), 0);
   251		return EAX_EDX_VAL(val, low, high);
   252	}
   253	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29309 bytes --]