Re: amd64 profiling kernel build failure

[Ryo Shimizu <ryo%nerv.org@localhost>]

Hi all

>Hi,
>
>On 2016/01/08 16:00, David Holland wrote:
>> On Fri, Jan 08, 2016 at 06:50:02AM +0000, David Holland wrote:
>>  >  > --- a/sys/kern/subr_prof.c
>>  >  > +++ b/sys/kern/subr_prof.c
>>  >  > @@ -48,6 +48,10 @@ __KERNEL_RCSID(0, "$NetBSD: subr_prof.c,v 1.47 2014/07/10 21:13:52 christos Exp
>>  >  >  #include <sys/malloc.h>
>>  >  >  #include <sys/gmon.h>
>>  >  >  
>>  >  > +#ifdef MULTIPROCESSOR
>>  >  > +__cpu_simple_lock_t __mcount_lock;
>>  >  > +#endif
>>  >  > +
>>  > 
>>  > This should be in an MD file. Not sure offhand which one.
>> 
>> Also, the i386 profile.h needs the same change as the amd64 one, so
>> the md file should probably be one in arch/x86/x86.

BTW, as far as I know other MULTIPROCESSOR arch also needs __mcount_lock, but none.
At least kernel profiling doesn't work on arm/MP.

therefore __mcount_lock should be moved to common/lib/libc/gmon/mcount.c
from machine/profile.h as below.


diff --git a/common/lib/libc/gmon/mcount.c b/common/lib/libc/gmon/mcount.c
index 128abe6..bf50572 100644
--- a/common/lib/libc/gmon/mcount.c
+++ b/common/lib/libc/gmon/mcount.c
@@ -82,6 +82,7 @@ __RCSID("$NetBSD: mcount.c,v 1.10 2012/03/20 16:21:41 matt Exp $");
 
 #include <sys/param.h>
 #include <sys/gmon.h>
+#include <sys/lock.h>
 
 #ifndef _KERNEL
 #include "reentrant.h"
@@ -93,6 +94,10 @@ extern struct gmonparam _gmondummy;
 struct gmonparam *_m_gmon_alloc(void);
 #endif
 
+#if defined(_KERNEL) && !defined(_RUMPKERNEL) && defined(MULTIPROCESSOR)
+__cpu_simple_lock_t __mcount_lock;
+#endif
+
 #ifndef __LINT__
 _MCOUNT_DECL(u_long, u_long)
 #ifdef _KERNEL
@@ -101,16 +106,6 @@ _MCOUNT_DECL(u_long, u_long)
     __used;
 #endif
 
-/* XXX: make these interfaces */
-#ifdef _RUMPKERNEL
-#undef MCOUNT_ENTER
-#define MCOUNT_ENTER
-#undef MCOUNT_EXIT
-#define MCOUNT_EXIT
-#undef MCOUNT
-#define MCOUNT
-#endif
-
 /*
  * mcount is called on entry to each function compiled with the profiling
  * switch set.  _mcount(), which is declared in a machine-dependent way
@@ -155,8 +150,12 @@ _MCOUNT_DECL(u_long frompc, u_long selfpc)
 	 */
 	if (p->state != GMON_PROF_ON)
 		return;
-#ifdef _KERNEL
+#if defined(_KERNEL) && !defined(_RUMPKERNEL)
 	MCOUNT_ENTER;
+#ifdef MULTIPROCESSOR
+	__cpu_simple_lock(&__mcount_lock);
+	__insn_barrier();
+#endif
 #endif
 	p->state = GMON_PROF_BUSY;
 	/*
@@ -250,7 +249,11 @@ _MCOUNT_DECL(u_long frompc, u_long selfpc)
 	}
 done:
 	p->state = GMON_PROF_ON;
-#ifdef _KERNEL
+#if defined(_KERNEL) && !defined(_RUMPKERNEL)
+#ifdef MULTIPROCESSOR
+	__insn_barrier();
+	__cpu_simple_unlock(&__mcount_lock);
+#endif
 	MCOUNT_EXIT;
 #endif
 	return;
diff --git a/sys/arch/amd64/include/profile.h b/sys/arch/amd64/include/profile.h
index f760594..3ff4d2d 100644
--- a/sys/arch/amd64/include/profile.h
+++ b/sys/arch/amd64/include/profile.h
@@ -34,7 +34,6 @@
 #ifdef __x86_64__
 
 #ifdef _KERNEL_OPT
-#include "opt_multiprocessor.h"
 #include "opt_xen.h"
 #endif
 
@@ -84,27 +83,6 @@ __asm(" .globl __mcount		\n"			\
 
 
 #ifdef _KERNEL
-#ifdef MULTIPROCESSOR
-__cpu_simple_lock_t __mcount_lock;
-
-static inline void
-MCOUNT_ENTER_MP(void)
-{
-	__cpu_simple_lock(&__mcount_lock);
-	__insn_barrier();
-}
-
-static inline void
-MCOUNT_EXIT_MP(void)
-{
-	__insn_barrier();
-	__mcount_lock = __SIMPLELOCK_UNLOCKED;
-}
-#else
-#define MCOUNT_ENTER_MP()
-#define MCOUNT_EXIT_MP()
-#endif
-
 #ifdef XEN
 static inline void
 mcount_disable_intr(void)
@@ -150,14 +128,10 @@ mcount_write_psl(u_long ef)
 }
 
 #endif /* XEN */
-#define	MCOUNT_ENTER							\
-	s = (int)mcount_read_psl();					\
-	mcount_disable_intr();						\
-	MCOUNT_ENTER_MP();
-
-#define	MCOUNT_EXIT							\
-	MCOUNT_EXIT_MP();						\
-	mcount_write_psl(s);
+
+#define MCOUNT_ENTER	\
+	do { s = (int)mcount_read_psl(); mcount_disable_intr(); } while (0)
+#define MCOUNT_EXIT	do { mcount_write_psl(s); } while (0)
 
 #endif /* _KERNEL */
 
diff --git a/sys/arch/i386/include/profile.h b/sys/arch/i386/include/profile.h
index d49e95a..923aa33 100644
--- a/sys/arch/i386/include/profile.h
+++ b/sys/arch/i386/include/profile.h
@@ -31,13 +31,8 @@
  *	@(#)profile.h	8.1 (Berkeley) 6/11/93
  */
 
-#ifdef _KERNEL_OPT
-#include "opt_multiprocessor.h"
-#endif
-
 #ifdef _KERNEL
 #include <machine/cpufunc.h>
-#include <machine/lock.h>
 #endif
 
 #define	_MCOUNT_DECL static __inline void _mcount
@@ -83,27 +78,6 @@ mcount(void)								\
 }
 
 #ifdef _KERNEL
-#ifdef MULTIPROCESSOR
-__cpu_simple_lock_t __mcount_lock;
-
-static inline void
-MCOUNT_ENTER_MP(void)
-{
-	__cpu_simple_lock(&__mcount_lock);
-	__insn_barrier();
-}
-
-static inline void
-MCOUNT_EXIT_MP(void)
-{
-	__insn_barrier();
-	__mcount_lock = __SIMPLELOCK_UNLOCKED;
-}
-#else
-#define MCOUNT_ENTER_MP()
-#define MCOUNT_EXIT_MP()
-#endif
-
 static inline void
 mcount_disable_intr(void)
 {
@@ -125,13 +99,8 @@ mcount_write_psl(u_long ef)
 	__asm volatile("pushl %0; popfl" : : "r" (ef));
 }
 
-#define	MCOUNT_ENTER							\
-	s = (int)mcount_read_psl();					\
-	mcount_disable_intr();						\
-	MCOUNT_ENTER_MP();
-
-#define	MCOUNT_EXIT							\
-	MCOUNT_EXIT_MP();						\
-	mcount_write_psl(s);
+#define MCOUNT_ENTER	\
+	do { s = (int)mcount_read_psl(); mcount_disable_intr(); } while (0)
+#define MCOUNT_EXIT	do { mcount_write_psl(s); } while (0)
 
 #endif /* _KERNEL */


--
ryo shimizu