Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 14 Jul 2014 11:10:50 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Mark Johnston <markj@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r268600 - in head/sys: amd64/amd64 cddl/dev/dtrace/amd64 cddl/dev/dtrace/i386 cddl/dev/dtrace/mips cddl/dev/dtrace/powerpc i386/i386 mips/mips powerpc/aim sys
Message-ID:  <20140714081050.GE93733@kib.kiev.ua>
In-Reply-To: <201407140438.s6E4cHCh016707@svn.freebsd.org>
References:  <201407140438.s6E4cHCh016707@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--CPxrXXhckaW0WC23
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Jul 14, 2014 at 04:38:17AM +0000, Mark Johnston wrote:
> Author: markj
> Date: Mon Jul 14 04:38:17 2014
> New Revision: 268600
> URL: http://svnweb.freebsd.org/changeset/base/268600
>=20
> Log:
>   Invoke the DTrace trap handler before calling trap() on amd64. This mat=
ches
>   the upstream implementation and helps ensure that a trap induced by tra=
cing
>   fbt::trap:entry is handled without recursively generating another trap.
>  =20
>   This makes it possible to run most (but not all) of the DTrace tests un=
der
>   common/safety/ without triggering a kernel panic.
>  =20
>   Submitted by:	Anton Rang <anton.rang@isilon.com> (original version)
>   Phabric:	D95
>=20
> Modified:
>   head/sys/amd64/amd64/exception.S
>   head/sys/amd64/amd64/trap.c
>   head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c
>   head/sys/cddl/dev/dtrace/i386/dtrace_subr.c
>   head/sys/cddl/dev/dtrace/mips/dtrace_subr.c
>   head/sys/cddl/dev/dtrace/powerpc/dtrace_subr.c
>   head/sys/i386/i386/trap.c
>   head/sys/mips/mips/trap.c
>   head/sys/powerpc/aim/trap.c
>   head/sys/sys/dtrace_bsd.h
>=20
> Modified: head/sys/amd64/amd64/exception.S
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/amd64/amd64/exception.S	Mon Jul 14 00:16:49 2014	(r268599)
> +++ head/sys/amd64/amd64/exception.S	Mon Jul 14 04:38:17 2014	(r268600)
> @@ -228,7 +228,24 @@ alltraps_pushregs_no_rdi:
>  	.type	calltrap,@function
>  calltrap:
>  	movq	%rsp,%rdi
> +#ifdef KDTRACE_HOOKS
> +	/*
> +	 * Give DTrace a chance to vet this trap and skip the call to trap() if
> +	 * it turns out that it was caused by a DTrace probe.
> +	 */
> +	movq	dtrace_trap_func,%rax
> +	testq	%rax,%rax
> +	je	skiphook
> +	call	*%rax
> +	testq	%rax,%rax
> +	jne	skiptrap
> +	movq	%rsp,%rdi
> +skiphook:
> +#endif
>  	call	trap
Why is this fragment done in asm ?  I see it relatively easy to
either move to the start of trap(), or create a new wrapper around
current trap() which calls dtrace_trap_func conditionally, and then
resorts to the old trap.

> +#ifdef KDTRACE_HOOKS
> +skiptrap:
> +#endif
>  	MEXITCOUNT
>  	jmp	doreti			/* Handle any pending ASTs */
> =20
>=20
> Modified: head/sys/amd64/amd64/trap.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/amd64/amd64/trap.c	Mon Jul 14 00:16:49 2014	(r268599)
> +++ head/sys/amd64/amd64/trap.c	Mon Jul 14 04:38:17 2014	(r268600)
> @@ -218,18 +218,6 @@ trap(struct trapframe *frame)
>  		goto out;
>  	}
> =20
> -#ifdef KDTRACE_HOOKS
> -	/*
> -	 * A trap can occur while DTrace executes a probe. Before
> -	 * executing the probe, DTrace blocks re-scheduling and sets
> -	 * a flag in its per-cpu flags to indicate that it doesn't
> -	 * want to fault. On returning from the probe, the no-fault
> -	 * flag is cleared and finally re-scheduling is enabled.
> -	 */
> -	if (dtrace_trap_func !=3D NULL && (*dtrace_trap_func)(frame, type))
> -		goto out;
> -#endif
> -
>  	if ((frame->tf_rflags & PSL_I) =3D=3D 0) {
>  		/*
>  		 * Buggy application or kernel code has disabled
>=20
> Modified: head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c	Mon Jul 14 00:16:49 2014=
	(r268599)
> +++ head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c	Mon Jul 14 04:38:17 2014=
	(r268600)
> @@ -462,29 +462,27 @@ dtrace_gethrestime(void)
>  	return (current_time.tv_sec * 1000000000ULL + current_time.tv_nsec);
>  }
> =20
> -/* Function to handle DTrace traps during probes. See amd64/amd64/trap.c=
 */
> +/*
> + * Function to handle DTrace traps during probes. See amd64/amd64/except=
ion.S.
> + */
>  int
> -dtrace_trap(struct trapframe *frame, u_int type)
> +dtrace_trap(struct trapframe *frame)
>  {
>  	/*
>  	 * A trap can occur while DTrace executes a probe. Before
>  	 * executing the probe, DTrace blocks re-scheduling and sets
> -	 * a flag in it's per-cpu flags to indicate that it doesn't
> +	 * a flag in its per-cpu flags to indicate that it doesn't
>  	 * want to fault. On returning from the probe, the no-fault
>  	 * flag is cleared and finally re-scheduling is enabled.
>  	 *
>  	 * Check if DTrace has enabled 'no-fault' mode:
> -	 *
>  	 */
>  	if ((cpu_core[curcpu].cpuc_dtrace_flags & CPU_DTRACE_NOFAULT) !=3D 0) {
>  		/*
>  		 * There are only a couple of trap types that are expected.
>  		 * All the rest will be handled in the usual way.
>  		 */
> -		switch (type) {
> -		/* Privilieged instruction fault. */
> -		case T_PRIVINFLT:
> -			break;
> +		switch (frame->tf_trapno) {
>  		/* General protection fault. */
>  		case T_PROTFLT:
>  			/* Flag an illegal operation. */
>=20
> Modified: head/sys/cddl/dev/dtrace/i386/dtrace_subr.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/cddl/dev/dtrace/i386/dtrace_subr.c	Mon Jul 14 00:16:49 2014	=
(r268599)
> +++ head/sys/cddl/dev/dtrace/i386/dtrace_subr.c	Mon Jul 14 04:38:17 2014	=
(r268600)
> @@ -473,24 +473,23 @@ dtrace_gethrestime(void)
> =20
>  /* Function to handle DTrace traps during probes. See i386/i386/trap.c */
>  int
> -dtrace_trap(struct trapframe *frame, u_int type)
> +dtrace_trap(struct trapframe *frame)
>  {
>  	/*
>  	 * A trap can occur while DTrace executes a probe. Before
>  	 * executing the probe, DTrace blocks re-scheduling and sets
> -	 * a flag in it's per-cpu flags to indicate that it doesn't
> +	 * a flag in its per-cpu flags to indicate that it doesn't
>  	 * want to fault. On returning from the probe, the no-fault
>  	 * flag is cleared and finally re-scheduling is enabled.
>  	 *
>  	 * Check if DTrace has enabled 'no-fault' mode:
> -	 *
>  	 */
>  	if ((cpu_core[curcpu].cpuc_dtrace_flags & CPU_DTRACE_NOFAULT) !=3D 0) {
>  		/*
>  		 * There are only a couple of trap types that are expected.
>  		 * All the rest will be handled in the usual way.
>  		 */
> -		switch (type) {
> +		switch (frame->tf_trapno) {
>  		/* General protection fault. */
>  		case T_PROTFLT:
>  			/* Flag an illegal operation. */
>=20
> Modified: head/sys/cddl/dev/dtrace/mips/dtrace_subr.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/cddl/dev/dtrace/mips/dtrace_subr.c	Mon Jul 14 00:16:49 2014	=
(r268599)
> +++ head/sys/cddl/dev/dtrace/mips/dtrace_subr.c	Mon Jul 14 04:38:17 2014	=
(r268600)
> @@ -137,17 +137,20 @@ dtrace_gethrestime(void)
> =20
>  /* Function to handle DTrace traps during probes. See amd64/amd64/trap.c=
 */
>  int
> -dtrace_trap(struct trapframe *frame, u_int type)
> +dtrace_trap(struct trapframe *frame)
>  {
> +	u_int type;
> +
> +	type =3D (trapframe->cause & MIPS_CR_EXC_CODE) >> MIPS_CR_EXC_CODE_SHIF=
T;
> +
>  	/*
>  	 * A trap can occur while DTrace executes a probe. Before
>  	 * executing the probe, DTrace blocks re-scheduling and sets
> -	 * a flag in it's per-cpu flags to indicate that it doesn't
> +	 * a flag in its per-cpu flags to indicate that it doesn't
>  	 * want to fault. On returning from the probe, the no-fault
>  	 * flag is cleared and finally re-scheduling is enabled.
>  	 *
>  	 * Check if DTrace has enabled 'no-fault' mode:
> -	 *
>  	 */
>  	if ((cpu_core[curcpu].cpuc_dtrace_flags & CPU_DTRACE_NOFAULT) !=3D 0) {
>  		/*
>=20
> Modified: head/sys/cddl/dev/dtrace/powerpc/dtrace_subr.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/cddl/dev/dtrace/powerpc/dtrace_subr.c	Mon Jul 14 00:16:49 20=
14	(r268599)
> +++ head/sys/cddl/dev/dtrace/powerpc/dtrace_subr.c	Mon Jul 14 04:38:17 20=
14	(r268600)
> @@ -262,24 +262,23 @@ dtrace_gethrestime(void)
> =20
>  /* Function to handle DTrace traps during probes. See powerpc/powerpc/tr=
ap.c */
>  int
> -dtrace_trap(struct trapframe *frame, u_int type)
> +dtrace_trap(struct trapframe *frame)
>  {
>  	/*
>  	 * A trap can occur while DTrace executes a probe. Before
>  	 * executing the probe, DTrace blocks re-scheduling and sets
> -	 * a flag in it's per-cpu flags to indicate that it doesn't
> +	 * a flag in its per-cpu flags to indicate that it doesn't
>  	 * want to fault. On returning from the probe, the no-fault
>  	 * flag is cleared and finally re-scheduling is enabled.
>  	 *
>  	 * Check if DTrace has enabled 'no-fault' mode:
> -	 *
>  	 */
>  	if ((cpu_core[curcpu].cpuc_dtrace_flags & CPU_DTRACE_NOFAULT) !=3D 0) {
>  		/*
>  		 * There are only a couple of trap types that are expected.
>  		 * All the rest will be handled in the usual way.
>  		 */
> -		switch (type) {
> +		switch (frame->exc) {
>  		/* Page fault. */
>  		case EXC_DSI:
>  		case EXC_DSE:
>=20
> Modified: head/sys/i386/i386/trap.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/i386/i386/trap.c	Mon Jul 14 00:16:49 2014	(r268599)
> +++ head/sys/i386/i386/trap.c	Mon Jul 14 04:38:17 2014	(r268600)
> @@ -246,7 +246,7 @@ trap(struct trapframe *frame)
>  	 * flag is cleared and finally re-scheduling is enabled.
>  	 */
>  	if ((type =3D=3D T_PROTFLT || type =3D=3D T_PAGEFLT) &&
> -	    dtrace_trap_func !=3D NULL && (*dtrace_trap_func)(frame, type))
> +	    dtrace_trap_func !=3D NULL && (*dtrace_trap_func)(frame))
>  		goto out;
>  #endif
> =20
>=20
> Modified: head/sys/mips/mips/trap.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/mips/mips/trap.c	Mon Jul 14 00:16:49 2014	(r268599)
> +++ head/sys/mips/mips/trap.c	Mon Jul 14 04:38:17 2014	(r268600)
> @@ -605,7 +605,7 @@ trap(struct trapframe *trapframe)
>  	/*
>  	 * A trap can occur while DTrace executes a probe. Before
>  	 * executing the probe, DTrace blocks re-scheduling and sets
> -	 * a flag in it's per-cpu flags to indicate that it doesn't
> +	 * a flag in its per-cpu flags to indicate that it doesn't
>  	 * want to fault. On returning from the probe, the no-fault
>  	 * flag is cleared and finally re-scheduling is enabled.
>  	 *
> @@ -618,7 +618,7 @@ trap(struct trapframe *trapframe)
>  	 * XXXDTRACE: add pid probe handler here (if ever)
>  	 */
>  	if (!usermode) {
> -		if (dtrace_trap_func !=3D NULL && (*dtrace_trap_func)(trapframe, type))
> +		if (dtrace_trap_func !=3D NULL && (*dtrace_trap_func)(trapframe))
>  			return (trapframe->pc);
>  	}
>  #endif
>=20
> Modified: head/sys/powerpc/aim/trap.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/powerpc/aim/trap.c	Mon Jul 14 00:16:49 2014	(r268599)
> +++ head/sys/powerpc/aim/trap.c	Mon Jul 14 04:38:17 2014	(r268600)
> @@ -167,7 +167,7 @@ trap(struct trapframe *frame)
>  	/*
>  	 * A trap can occur while DTrace executes a probe. Before
>  	 * executing the probe, DTrace blocks re-scheduling and sets
> -	 * a flag in it's per-cpu flags to indicate that it doesn't
> +	 * a flag in its per-cpu flags to indicate that it doesn't
>  	 * want to fault. On returning from the probe, the no-fault
>  	 * flag is cleared and finally re-scheduling is enabled.
>  	 *
> @@ -176,7 +176,7 @@ trap(struct trapframe *frame)
>  	 * handled the trap and modified the trap frame so that this
>  	 * function can return normally.
>  	 */
> -	if (dtrace_trap_func !=3D NULL && (*dtrace_trap_func)(frame, type))
> +	if (dtrace_trap_func !=3D NULL && (*dtrace_trap_func)(frame))
>  		return;
>  #endif
> =20
>=20
> Modified: head/sys/sys/dtrace_bsd.h
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/sys/dtrace_bsd.h	Mon Jul 14 00:16:49 2014	(r268599)
> +++ head/sys/sys/dtrace_bsd.h	Mon Jul 14 04:38:17 2014	(r268600)
> @@ -48,15 +48,14 @@ extern cyclic_clock_func_t	cyclic_clock_
> =20
>  void clocksource_cyc_set(const struct bintime *t);
> =20
> +int dtrace_trap(struct trapframe *);
> +
>  /*
>   * The dtrace module handles traps that occur during a DTrace probe.
>   * This type definition is used in the trap handler to provide a
> - * hook for the dtrace module to register it's handler with.
> + * hook for the dtrace module to register its handler with.
>   */
> -typedef int (*dtrace_trap_func_t)(struct trapframe *, u_int);
> -
> -int	dtrace_trap(struct trapframe *, u_int);
> -
> +typedef int (*dtrace_trap_func_t)(struct trapframe *);
>  extern dtrace_trap_func_t	dtrace_trap_func;
> =20
>  /*

--CPxrXXhckaW0WC23
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJTw5CKAAoJEJDCuSvBvK1Be2IP/32LvxHPUVLYbJnJGBYKV/Sf
g6VAyGh78B5H+tzG1fRvoaeWJ9PXFd1qKX5+cEmLjJTbF1/foZ5YKZkd3iT0w+vU
gfKvuTTNwfuqhanDQKCxBwexKs2L0YdBQo1JpTVqdNDHg1rKgTmsBAFFLVM/XVAf
iiq85BqmchrLBVC2fO7CzN35KJRw4hrjHijj6Rn0A7gP6RL2s5LJPa6VeGYmtHlg
AUNauJBdn4wWEcFD20gjXxhW/FWG4+4qo/QfMlo5ErU1PF28ouFVk1xe/MF46KF+
1E5FhF0jHz6tdLZjk3y1NLqVoVLPgsZPSnWTs/Xtb/7GGr3tdtyDrSu394rcatlQ
CBq9siKTRdSsHZ0pO5IF92B3dt8AoXTIKdDHWVcPSrZOgTvI4ubeSUaluq7iInJy
k+eB8xVPMGS14HnQrOlJ9XBIzdUJXf4pcCZc7Fj9q3IGTVAdxQtAv/DohJdC4CEd
SyPcV6ISilPk0zGIKqiB5Rta3s8jG6J3SILIcUmEwXz4DqC+Dfmwu0QAmnjKUgHo
bQo+0VqF+D9ycBlUl+AlvGjwf4lylRBAEXDzNgPhb8Ox6R+W+h4uXhE1lAlkae70
PEAtdj3PrP5bcs4gjnmYjGgfnOfAMLd9VQiNX0iYTbqaPnGM03DUQj4YU1X8oEAO
I1rkA/o/45NKdMJEgnoK
=bioD
-----END PGP SIGNATURE-----

--CPxrXXhckaW0WC23--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140714081050.GE93733>