Date: Thu, 20 Jun 2024 16:45:43 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 70c712a86da5 - main - sdt: Support fetching the probe sixth argument with MI machinery Message-ID: <202406201645.45KGjhq1048084@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=70c712a86da5d9760074b84dee15e229190bbd11 commit 70c712a86da5d9760074b84dee15e229190bbd11 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2024-06-20 16:40:25 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2024-06-20 16:40:25 +0000 sdt: Support fetching the probe sixth argument with MI machinery SDT calls dtrace_probe() directly, and this can be used to pass up to five probe arguments directly. To pass the sixth argument (SDT currently doesn't support more than this), we use a hack: just add additional parameters to the call and cast dtrace_probe accordingly. This happens to work on amd64, but doesn't work in general. Modify SDT to call dtrace_probe() after storing arguments beyond the first five in thread-local storage. Implement sdt_getargval() to fetch extra argument values this way. An alternative would be to use invop handlers instead and make sdt_probe_func point to a breakpoint instruction, so that one can extract arguments using the breakpoint exception trapframe, but this makes the providers more expensive when enabled and doesn't seem justified. This approach works well unless we want to add more than one or two more parameters to SDT probes, which seems unlikely at present. In particular, this fixes fetching the last argument of most ip and tcp probes on arm64. Reported by: rwatson Reviewed by: Domagoj Stolfa MFC after: 1 month Sponsored by: Innovate UK Differential Revision: https://reviews.freebsd.org/D45648 --- sys/cddl/dev/dtrace/dtrace_cddl.h | 3 ++- sys/cddl/dev/sdt/sdt.c | 35 +++++++++++++++++++++++++++++++---- sys/kern/kern_sdt.c | 8 ++++---- sys/sys/sdt.h | 5 +---- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/sys/cddl/dev/dtrace/dtrace_cddl.h b/sys/cddl/dev/dtrace/dtrace_cddl.h index 42a4721fbb90..c151c8cf9660 100644 --- a/sys/cddl/dev/dtrace/dtrace_cddl.h +++ b/sys/cddl/dev/dtrace/dtrace_cddl.h @@ -79,7 +79,7 @@ typedef struct kdtrace_thread { #ifdef __amd64__ uintptr_t td_dtrace_regv; #endif - uint64_t td_hrtime; /* Last time on cpu. */ + uintptr_t td_dtrace_sdt_arg[1]; /* Space for extra SDT args */ void *td_dtrace_sscr; /* Saved scratch space location. */ void *td_systrace_args; /* syscall probe arguments. */ uint64_t td_fasttrap_tp_gen; /* Tracepoint hash table gen. */ @@ -110,6 +110,7 @@ typedef struct kdtrace_thread { #define t_dtrace_scrpc td_dtrace->td_dtrace_scrpc #define t_dtrace_astpc td_dtrace->td_dtrace_astpc #define t_dtrace_regv td_dtrace->td_dtrace_regv +#define t_dtrace_sdt_arg td_dtrace->td_dtrace_sdt_arg #define t_dtrace_sscr td_dtrace->td_dtrace_sscr #define t_dtrace_systrace_args td_dtrace->td_systrace_args #define t_fasttrap_tp_gen td_dtrace->td_fasttrap_tp_gen diff --git a/sys/cddl/dev/sdt/sdt.c b/sys/cddl/dev/sdt/sdt.c index 51fa0432437c..461f454186a3 100644 --- a/sys/cddl/dev/sdt/sdt.c +++ b/sys/cddl/dev/sdt/sdt.c @@ -58,8 +58,11 @@ #include <sys/dtrace.h> #include <sys/dtrace_bsd.h> +#include <cddl/dev/dtrace/dtrace_cddl.h> + /* DTrace methods. */ static void sdt_getargdesc(void *, dtrace_id_t, void *, dtrace_argdesc_t *); +static uint64_t sdt_getargval(void *, dtrace_id_t, void *, int, int); static void sdt_provide_probes(void *, dtrace_probedesc_t *); static void sdt_destroy(void *, dtrace_id_t, void *); static void sdt_enable(void *, dtrace_id_t, void *); @@ -93,7 +96,7 @@ static dtrace_pops_t sdt_pops = { .dtps_suspend = NULL, .dtps_resume = NULL, .dtps_getargdesc = sdt_getargdesc, - .dtps_getargval = NULL, + .dtps_getargval = sdt_getargval, .dtps_usermode = NULL, .dtps_destroy = sdt_destroy, }; @@ -321,6 +324,23 @@ sdt_getargdesc(void *arg, dtrace_id_t id, void *parg, dtrace_argdesc_t *desc) } } +/* + * Fetch arguments beyond the first five passed directly to dtrace_probe(). + * FreeBSD's SDT implement currently only supports up to 6 arguments, so we just + * need to handle arg5 here. + */ +static uint64_t +sdt_getargval(void *arg __unused, dtrace_id_t id __unused, + void *parg __unused, int argno, int aframes __unused) +{ + if (argno != 5) { + DTRACE_CPUFLAG_SET(CPU_DTRACE_ILLOP); + return (0); + } else { + return (curthread->t_dtrace_sdt_arg[argno - 5]); + } +} + static void sdt_destroy(void *arg, dtrace_id_t id, void *parg) { @@ -449,14 +469,21 @@ sdt_load_probes_cb(linker_file_t lf, void *arg __unused) return (0); } +static void +sdt_dtrace_probe(dtrace_id_t id, uintptr_t arg0, uintptr_t arg1, + uintptr_t arg2, uintptr_t arg3, uintptr_t arg4, uintptr_t arg5) +{ + curthread->t_dtrace_sdt_arg[0] = arg5; + dtrace_probe(id, arg0, arg1, arg2, arg3, arg4); +} + static void sdt_load(void) { TAILQ_INIT(&sdt_prov_list); - sdt_probe_func = dtrace_probe; - sdt_probe6_func = (sdt_probe6_func_t)dtrace_probe; + sdt_probe_func = sdt_dtrace_probe; sdt_kld_load_tag = EVENTHANDLER_REGISTER(kld_load, sdt_kld_load, NULL, EVENTHANDLER_PRI_ANY); @@ -482,7 +509,6 @@ sdt_unload(void) EVENTHANDLER_DEREGISTER(kld_unload_try, sdt_kld_unload_try_tag); sdt_probe_func = sdt_probe_stub; - sdt_probe6_func = (sdt_probe6_func_t)sdt_probe_stub; TAILQ_FOREACH_SAFE(prov, &sdt_prov_list, prov_entry, tmp) { ret = dtrace_unregister(prov->id); @@ -515,3 +541,4 @@ SYSUNINIT(sdt_unload, SI_SUB_DTRACE_PROVIDER, SI_ORDER_ANY, sdt_unload, NULL); DEV_MODULE(sdt, sdt_modevent, NULL); MODULE_VERSION(sdt, 1); MODULE_DEPEND(sdt, dtrace, 1, 1, 1); +MODULE_DEPEND(sdt, opensolaris, 1, 1, 1); diff --git a/sys/kern/kern_sdt.c b/sys/kern/kern_sdt.c index b7213d2051fc..9704326299e6 100644 --- a/sys/kern/kern_sdt.c +++ b/sys/kern/kern_sdt.c @@ -28,6 +28,7 @@ #include <sys/param.h> #include <sys/systm.h> #include <sys/kdb.h> +#include <sys/proc.h> #include <sys/sdt.h> SDT_PROVIDER_DEFINE(sdt); @@ -37,7 +38,6 @@ SDT_PROVIDER_DEFINE(sdt); * dtrace_probe() when it loads. */ sdt_probe_func_t sdt_probe_func = sdt_probe_stub; -sdt_probe6_func_t sdt_probe6_func = (sdt_probe6_func_t)sdt_probe_stub; volatile bool __read_frequently sdt_probes_enabled; /* @@ -48,7 +48,7 @@ volatile bool __read_frequently sdt_probes_enabled; void sdt_probe_stub(uint32_t id __unused, uintptr_t arg0 __unused, uintptr_t arg1 __unused, uintptr_t arg2 __unused, uintptr_t arg3 __unused, - uintptr_t arg4 __unused) + uintptr_t arg4 __unused, uintptr_t arg5 __unused) { printf("sdt_probe_stub: unexpectedly called\n"); kdb_backtrace(); @@ -58,12 +58,12 @@ void sdt_probe(uint32_t id, uintptr_t arg0, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3, uintptr_t arg4) { - sdt_probe_func(id, arg0, arg1, arg2, arg3, arg4); + sdt_probe_func(id, arg0, arg1, arg2, arg3, arg4, 0); } void sdt_probe6(uint32_t id, uintptr_t arg0, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3, uintptr_t arg4, uintptr_t arg5) { - sdt_probe6_func(id, arg0, arg1, arg2, arg3, arg4, arg5); + sdt_probe_func(id, arg0, arg1, arg2, arg3, arg4, arg5); } diff --git a/sys/sys/sdt.h b/sys/sys/sdt.h index 147d58c53ef4..0987f1cc19c3 100644 --- a/sys/sys/sdt.h +++ b/sys/sys/sdt.h @@ -419,15 +419,12 @@ __sdt_probe##uniq:; \ * way to avoid having to rely on CDDL code. */ typedef void (*sdt_probe_func_t)(uint32_t, uintptr_t arg0, uintptr_t arg1, - uintptr_t arg2, uintptr_t arg3, uintptr_t arg4); -typedef void (*sdt_probe6_func_t)(uint32_t, uintptr_t arg0, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3, uintptr_t arg4, uintptr_t arg5); /* * The 'sdt' provider will set it to dtrace_probe when it loads. */ extern sdt_probe_func_t sdt_probe_func; -extern sdt_probe6_func_t sdt_probe6_func; struct sdt_probe; struct sdt_provider; @@ -466,7 +463,7 @@ struct sdt_provider { }; void sdt_probe_stub(uint32_t, uintptr_t, uintptr_t, uintptr_t, uintptr_t, - uintptr_t); + uintptr_t, uintptr_t); SDT_PROVIDER_DECLARE(sdt);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202406201645.45KGjhq1048084>