--cdjqm/zyp5sS28aW
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Wed, Jul 16, 2014 at 12:11:23AM -0400, Mark Johnston wrote:
> On Mon, Jul 14, 2014 at 11:10:50AM +0300, Konstantin Belousov wrote:
> > 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=
matches
> > > the upstream implementation and helps ensure that a trap induced by=
tracing
> > > fbt::trap:entry is handled without recursively generating another t=
rap.
> > > =20
> > > This makes it possible to run most (but not all) of the DTrace test=
s under
> > > 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 (r26859=
9)
> > > +++ head/sys/amd64/amd64/exception.S Mon Jul 14 04:38:17 2014 (r26860=
0)
> > > @@ -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.
>=20
> You're right, it looks like this could be done in C by introducing a
> wrapper. I'm not sure how moving the conditional call to
> dtrace_trap_func() around within trap() would fix the original problem,
> though.
Ok.
>=20
> The diff below adds such a wrapper, and modifies DTrace to explicitly
> ignore it when creating function boundary probes. Additionally, trap()
> is marked __noinline to ensure that it can still be instrumented. I've
> named it "_trap" for now for lack of a better name, but that will need
> to change.
Looks fine. This should also fix the issue of kgdb and ddb not able to
unwind the trap frames.
>=20
> Thanks,
> -Mark
>=20
> diff --git a/sys/amd64/amd64/exception.S b/sys/amd64/amd64/exception.S
> index bb5fd56..3d34724 100644
> --- a/sys/amd64/amd64/exception.S
> +++ b/sys/amd64/amd64/exception.S
> @@ -228,24 +228,7 @@ 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
> -#ifdef KDTRACE_HOOKS
> -skiptrap:
> -#endif
> + call _trap
> MEXITCOUNT
> jmp doreti /* Handle any pending ASTs */
> =20
> diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c
> index d9203bc..545174a 100644
> --- a/sys/amd64/amd64/trap.c
> +++ b/sys/amd64/amd64/trap.c
> @@ -97,7 +97,8 @@ PMC_SOFT_DEFINE( , , page_fault, write);
> #include <sys/dtrace_bsd.h>
> #endif
> =20
> -extern void trap(struct trapframe *frame);
> +extern void _trap(struct trapframe *frame);
> +extern void __noinline trap(struct trapframe *frame);
> extern void syscall(struct trapframe *frame);
> void dblfault_handler(struct trapframe *frame);
> =20
> @@ -604,6 +605,19 @@ out:
> return;
> }
> =20
> +/*
> + * Ensure that we ignore any DTrace-induced faults. This function cannot
> + * be instrumented, so it cannot generate such faults itself.
> + */
> +void
> +_trap(struct trapframe *frame)
> +{
> +
> + if (dtrace_trap_func !=3D NULL && (*dtrace_trap_func)(frame))
> + return;
> + trap(frame);
> +}
> +
> static int
> trap_pfault(frame, usermode)
> struct trapframe *frame;
> diff --git a/sys/cddl/dev/fbt/fbt.c b/sys/cddl/dev/fbt/fbt.c
> index 7e256e7..0cc2db2 100644
> --- a/sys/cddl/dev/fbt/fbt.c
> +++ b/sys/cddl/dev/fbt/fbt.c
> @@ -232,13 +232,18 @@ fbt_provide_module_function(linker_file_t lf, int s=
ymindx,
> int size;
> u_int8_t *instr, *limit;
> =20
> - if (strncmp(name, "dtrace_", 7) =3D=3D 0 &&
> - strncmp(name, "dtrace_safe_", 12) !=3D 0) {
> + if ((strncmp(name, "dtrace_", 7) =3D=3D 0 &&
> + strncmp(name, "dtrace_safe_", 12) !=3D 0) ||
> + strcmp(name, "_trap") =3D=3D 0) {
> /*
> * Anything beginning with "dtrace_" may be called
> * from probe context unless it explicitly indicates
> * that it won't be called from probe context by
> * using the prefix "dtrace_safe_".
> + *
> + * Additionally, we avoid instrumenting _trap() to avoid the
> + * possibility of generating a fault in probe context before
> + * DTrace's fault handler is called.
> */
> return (0);
> }
--cdjqm/zyp5sS28aW
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBAgAGBQJTxjxUAAoJEJDCuSvBvK1BhnEQAKW4QFoXHHLVsVb8cY6biKd6
ldJQpVUMEK8vGU6y0lQLqc2Wf01IXqdrfln2UktFZpXkwXY4vONqwF3n4BCKZ+VL
F1Wpq6pKXLFSaFUalp1tuSUBzYsoNahNS5t4mf99OjT+7e5fgbbjOkIfoOAt9hU/
2DRX85WGDFavmoDc2JmB27nvDkcgprSDOYC4vj0J3FMsYJ0MEpw26xbwW7ca1t15
/NLSoC4SSfEF3PnyC7t33db+GqzXERQTeJZH+dO81yjLdfstbhW3fZ5LS6tApbkW
NVPu1yKkVAfk2m/AwGN5AJafU32D9rHqEtZxYmTQdDNY8ffASumCsktu+ZhJY/XL
8Fdhh7OlrR3YB+InUalikL/Nk5ExPFAddyZp9zjfrvle5QeZiSYmopLMvnLBqMyy
ai002i/Og9ke1tcT23BYvnuSE7Zkj7sS3Y95LkedSNx7LaVB2Mh3h3PY2bmyHzdX
2Odlfc9n8F0BM9FslnsyUxO0EXe+Kr/Y02NoVt6ybHsON6rCFndpxcxHeIGettOM
q68XDXm/jHANfVfbHAxEPr2HlcJ2PqFI6+qhcn5wFgF1bmcJ9FRe6xa8kB9vZs2w
ySgFP/kFflbMvrRqvFCmwfn4rpiR1+eLqhJrrhtjbxZr8o9SJ45WiK1XaqrJ+PTW
9r2HU8RAALgM4vjKdq42
=COs1
-----END PGP SIGNATURE-----
--cdjqm/zyp5sS28aW--