[DTrace-devel] [PATCH 3/7] dtrace: add default options and apply path options earlier
Eugene Loh
eugene.loh at oracle.com
Tue May 9 04:02:19 UTC 2023
I'm having problems getting my head around this patch (and therefore
subsequent patches). Maybe I'll try a few questions:
1) What does procfspath have to do with this patch? Was procfspath
broken? If so, is there now a test to show that it's fixed?
2) Will this patch break other consumers? The new "bt" option category
does change how DTrace has been documenting options... but maybe that
does not need to be exposed?
3) How about other approaches? Like, can dtrace_open() just set up the
dtp, deferring other work (e.g., that requires options to have been set)
until dtrace_init()?
On 5/2/23 13:12, Nick Alcock via DTrace-devel wrote:
> Various DTrace options apply to path searches which may potentially be
> performed early in init (module paths, CTF loading etc). Unfortunately
> because the options are only set after dtrace_open() returns, they
> cannot take effect. After the last commit this applies not only to the
> procfspath (as before) but to the modpath and ctfpath as well.
>
> This is getting too much. Migrate such options into a new
> _dtrace_btoptions and extend dtrace_setopt so that if you call it with a
> NULL dtp it adds the passed-in option to a (never-deallocated, static
> linkage) list of default options. Those default options found in the
> _dtrace_btoptions array are set early in dt_vopen(), before the
> dtrace_update() or CTF init; the rest get applied at the end, leaving
> them applied at the same time as they currently are.
>
> Move dtrace_open later in cmd/dtrace.c, to after pass 2 over the
> command-line options has pulled the DTrace options in, making them all
> "default" by this definition and applying boot-time ones earlier.
>
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
> cmd/dtrace.c | 27 ++++-----
> libdtrace/dt_impl.h | 1 +
> libdtrace/dt_open.c | 11 ++++
> libdtrace/dt_options.c | 124 ++++++++++++++++++++++++++++++++++++++---
> libdtrace/dtrace.h | 4 ++
> 5 files changed, 145 insertions(+), 22 deletions(-)
>
> diff --git a/cmd/dtrace.c b/cmd/dtrace.c
> index e7ca9e4c9c2a7..8af798774953e 100644
> --- a/cmd/dtrace.c
> +++ b/cmd/dtrace.c
> @@ -1081,15 +1081,8 @@ main(int argc, char *argv[])
> }
>
> /*
> - * Open libdtrace.
> - */
> - g_dtp = dtrace_open(DTRACE_VERSION, g_oflags, &err);
> - if (g_dtp == NULL)
> - fatal("failed to initialize dtrace: %s\n",
> - dtrace_errmsg(NULL, err));
> -
> - /*
> - * Set default options.
> + * Set default options. Because g_dtp is NULL these go into a cache of
> + * default options to be applied at dtrace_open time.
> */
> dtrace_setopt(g_dtp, "bufsize", "4m");
> dtrace_setopt(g_dtp, "aggsize", "4m");
> @@ -1123,10 +1116,10 @@ main(int argc, char *argv[])
> }
>
> /*
> - * Now that we have libdtrace open, make a second pass through argv[]
> - * to perform any dtrace_setopt() calls and change any compiler flags.
> - * We also accumulate any program specifications into our g_cmdv[] at
> - * this time; these will compiled as part of the fourth processing pass.
> + * Make a second pass through argv[] to perform any dtrace_setopt()
> + * calls and change any compiler flags. We also accumulate any program
> + * specifications into our g_cmdv[] at this time; these will compiled as
> + * part of the fourth processing pass.
> */
> optind = 1;
> while ((c = getopt(argc, argv, DTRACE_OPTSTR)) != EOF) {
> @@ -1282,6 +1275,14 @@ main(int argc, char *argv[])
> "testing", g_pname);
> }
>
> + /*
> + * Open libdtrace.
> + */
> + g_dtp = dtrace_open(DTRACE_VERSION, g_oflags, &err);
> + if (g_dtp == NULL)
> + fatal("failed to initialize dtrace: %s\n",
> + dtrace_errmsg(NULL, err));
> +
> /*
> * In our third pass we handle any command-line options related to
> * grabbing or creating victim processes. The behavior of these calls
> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> index 581bb8a644e7c..6b2ca3f14d9f4 100644
> --- a/libdtrace/dt_impl.h
> +++ b/libdtrace/dt_impl.h
> @@ -734,6 +734,7 @@ extern void dt_buffered_destroy(dtrace_hdl_t *);
> extern uint64_t dt_stddev(uint64_t *, uint64_t);
>
> extern int dt_options_load(dtrace_hdl_t *);
> +extern void dt_apply_default_options(dtrace_hdl_t *, int boot_time);
>
> extern void dt_setcontext(dtrace_hdl_t *, const dtrace_probedesc_t *);
> extern void dt_endcontext(dtrace_hdl_t *);
> diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> index f0ef053f5274d..3f36823372910 100644
> --- a/libdtrace/dt_open.c
> +++ b/libdtrace/dt_open.c
> @@ -900,6 +900,12 @@ dt_vopen(int version, int flags, int *errp,
> return set_open_errno(dtp, errp, EDT_NOMEM);
> }
>
> + /*
> + * Apply the early boot-time options: these affect paths that are used
> + * during module and CTF loading.
> + */
> + dt_apply_default_options(dtp, B_TRUE);
> +
> /*
> * Update the module list and load the values for the macro variable
> * definitions according to the current process.
> @@ -1159,6 +1165,11 @@ dt_vopen(int version, int flags, int *errp,
>
> dt_bpf_init_helpers(dtp);
>
> + /*
> + * Apply any further default options.
> + */
> + dt_apply_default_options(dtp, B_FALSE);
> +
> return dtp;
> }
>
> diff --git a/libdtrace/dt_options.c b/libdtrace/dt_options.c
> index 1c32dc3d4c150..2787311b4dc54 100644
> --- a/libdtrace/dt_options.c
> +++ b/libdtrace/dt_options.c
> @@ -1061,6 +1061,17 @@ typedef struct dt_option {
> uintptr_t o_option;
> } dt_option_t;
>
> +/*
> + * Boot-time options. These are applied halfway through dtrace_open. They
> + * should usually be set on a NULL dtp so that they have a chance to take effect
> + * early.
> + */
> +static const dt_option_t _dtrace_btoptions[] = {
> + { "ctfpath", dt_opt_ctfa_path },
> + { "modpath", dt_opt_module_path },
> + { "procfspath", dt_opt_procfs_path }
> +};
> +
> /*
> * Compile-time options.
> */
> @@ -1074,7 +1085,6 @@ static const dt_option_t _dtrace_ctoptions[] = {
> { "cpphdrs", dt_opt_cpp_hdrs },
> { "cpppath", dt_opt_cpp_path },
> { "ctypes", dt_opt_ctypes },
> - { "ctfpath", dt_opt_ctfa_path },
> { "defaultargs", dt_opt_cflags, DTRACE_C_DEFARG },
> { "debug", dt_opt_debug },
> { "debugassert", dt_opt_debug_assert },
> @@ -1095,11 +1105,9 @@ static const dt_option_t _dtrace_ctoptions[] = {
> { "libdir", dt_opt_libdir },
> { "linkmode", dt_opt_linkmode },
> { "linktype", dt_opt_linktype },
> - { "modpath", dt_opt_module_path },
> { "nolibs", dt_opt_cflags, DTRACE_C_NOLIBS },
> { "pgmax", dt_opt_pgmax },
> { "preallocate", dt_opt_preallocate },
> - { "procfspath", dt_opt_procfs_path },
> { "pspec", dt_opt_cflags, DTRACE_C_PSPEC },
> { "stdc", dt_opt_stdc },
> { "strip", dt_opt_dflags, DTRACE_D_STRIP },
> @@ -1195,6 +1203,74 @@ dtrace_getopt(dtrace_hdl_t *dtp, const char *opt, dtrace_optval_t *val)
> return dt_set_errno(dtp, EDT_BADOPTNAME);
> }
>
> +typedef struct default_option {
> + dt_list_t list;
> + char *opt;
> + char *val;
> +} default_option_t;
> +static dt_list_t default_options;
> +
> +/*
> + * These default options are applied at the end of dtrace_open, with the
> + * exception of the boot-time options, which are applied in the middle, before
> + * the type system is set up.
> + *
> + * They are allocated permanently and cannot be overwritten or deleted, only
> + * supplanted by later options.
> + */
> +static int
> +set_default_option(const char *opt, const char *val)
> +{
> + default_option_t *optp;
> +
> + optp = calloc(1, sizeof(default_option_t));
> + if (optp == NULL)
> + return -1; /* errno is set for us. */
> +
> + optp->opt = strdup(opt);
> +
> + if (val)
> + optp->val = strdup(val);
> +
> + if (optp->opt == NULL || (optp->val == NULL && val != NULL)) {
> + free(optp->opt);
> + free(optp->val);
> + free(optp);
> + return -1; /* errno is set for us. */
> + }
> +
> + dt_list_append(&default_options, optp);
> + return 0;
> +}
> +
> +void
> +dt_apply_default_options(dtrace_hdl_t *dtp, int boot_time)
> +{
> + default_option_t *optp;
> + int err = 0;
> +
> + for (optp = dt_list_next(&default_options); optp != NULL;
> + optp = dt_list_next(optp)) {
> +
> + if (boot_time) {
> + const dt_option_t *op;
> +
> + for (op = _dtrace_btoptions; op->o_name != NULL; op++) {
> + if (strcmp(op->o_name, optp->opt) == 0) {
> + int this_err;
> +
> + this_err = op->o_func(dtp, optp->val, op->o_option);
> + if (err == 0)
> + err = this_err;
> + break;
> + }
> + }
> + continue;
> + } else
> + dtrace_setopt(dtp, optp->opt, optp->val);
> + }
> +}
> +
> int
> dtrace_setopt(dtrace_hdl_t *dtp, const char *opt, const char *val)
> {
> @@ -1203,6 +1279,14 @@ dtrace_setopt(dtrace_hdl_t *dtp, const char *opt, const char *val)
> if (opt == NULL)
> return dt_set_errno(dtp, EINVAL);
>
> + if (dtp == NULL)
> + return set_default_option(opt, val);
> +
> + for (op = _dtrace_btoptions; op->o_name != NULL; op++) {
> + if (strcmp(op->o_name, opt) == 0)
> + return op->o_func(dtp, val, op->o_option);
> + }
> +
> for (op = _dtrace_ctoptions; op->o_name != NULL; op++) {
> if (strcmp(op->o_name, opt) == 0)
> return op->o_func(dtp, val, op->o_option);
> @@ -1261,21 +1345,43 @@ dtrace_setoptenv(dtrace_hdl_t *dtp, const char *prefix)
> if (prefix == NULL)
> prefix = "DTRACE_OPT_";
>
> + for (op = _dtrace_btoptions; op->o_name != NULL; op++) {
> + if ((val = dt_opt_getenv_prefix(dtp, op->o_name,
> + prefix)) != NULL) {
> + if (dtp == NULL)
> + set_default_option(op->o_name, val);
> + else
> + op->o_func(dtp, val, op->o_option);
> + }
> + }
> +
> for (op = _dtrace_ctoptions; op->o_name != NULL; op++) {
> if ((val = dt_opt_getenv_prefix(dtp, op->o_name,
> - prefix)) != NULL)
> - op->o_func(dtp, val, op->o_option);
> + prefix)) != NULL) {
> + if (dtp == NULL)
> + set_default_option(op->o_name, val);
> + else
> + op->o_func(dtp, val, op->o_option);
> + }
> }
>
> for (op = _dtrace_drtoptions; op->o_name != NULL; op++) {
> if ((val = dt_opt_getenv_prefix(dtp, op->o_name,
> - prefix)) != NULL)
> - op->o_func(dtp, val, op->o_option);
> + prefix)) != NULL) {
> + if (dtp == NULL)
> + set_default_option(op->o_name, val);
> + else
> + op->o_func(dtp, val, op->o_option);
> + }
> }
>
> for (op = _dtrace_rtoptions; op->o_name != NULL; op++) {
> if ((val = dt_opt_getenv_prefix(dtp, op->o_name,
> - prefix)) != NULL)
> - op->o_func(dtp, val, op->o_option);
> + prefix)) != NULL) {
> + if (dtp == NULL)
> + set_default_option(op->o_name, val);
> + else
> + op->o_func(dtp, val, op->o_option);
> + }
> }
> }
> diff --git a/libdtrace/dtrace.h b/libdtrace/dtrace.h
> index 51c8e081d89c5..18080c5db5203 100644
> --- a/libdtrace/dtrace.h
> +++ b/libdtrace/dtrace.h
> @@ -54,6 +54,10 @@ extern const char *dtrace_errmsg(dtrace_hdl_t *dtp, int error);
> extern const char *dtrace_faultstr(dtrace_hdl_t *dtp, int fault);
> extern const char *dtrace_subrstr(dtrace_hdl_t *dtp, int subr);
>
> +/*
> + * The setopt functions can be called with a NULL dtp to apply that option
> + * at the end of all future dtrace_open calls.
> + */
> extern int dtrace_setopt(dtrace_hdl_t *dtp, const char *opt, const char *val);
> extern int dtrace_getopt(dtrace_hdl_t *dtp, const char *opt,
> dtrace_optval_t *val);
More information about the DTrace-devel
mailing list