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
Konstantin Belousov
kostikbel at gmail.com
Wed Jul 16 08:48:28 UTC 2014
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
> > >
> > > 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 trap.
> > >
> > > This makes it possible to run most (but not all) of the DTrace tests under
> > > common/safety/ without triggering a kernel panic.
> > >
> > > Submitted by: Anton Rang <anton.rang at isilon.com> (original version)
> > > Phabric: D95
> > >
> > > 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
> > >
> > > Modified: head/sys/amd64/amd64/exception.S
> > > ==============================================================================
> > > --- 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, at 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.
>
> 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.
>
> 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.
>
> Thanks,
> -Mark
>
> 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, at 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 */
>
> 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
>
> -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);
>
> @@ -604,6 +605,19 @@ out:
> return;
> }
>
> +/*
> + * 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 != 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 symindx,
> int size;
> u_int8_t *instr, *limit;
>
> - if (strncmp(name, "dtrace_", 7) == 0 &&
> - strncmp(name, "dtrace_safe_", 12) != 0) {
> + if ((strncmp(name, "dtrace_", 7) == 0 &&
> + strncmp(name, "dtrace_safe_", 12) != 0) ||
> + strcmp(name, "_trap") == 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);
> }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20140716/da320874/attachment.sig>
More information about the svn-src-all
mailing list