* Re: [DTrace-devel] [PATCH v2] print() action: identify ctf object used for print
[not found] ` <Zik/P8WUMubC5Mh7@oracle.com>
@ 2024-04-24 17:35 ` Kris Van Hees
0 siblings, 0 replies; only message in thread
From: Kris Van Hees @ 2024-04-24 17:35 UTC (permalink / raw)
To: dtrace; +Cc: Alan Maguire, dtrace-devel
On Wed, Apr 24, 2024 at 01:19:59PM -0400, Kris Van Hees via DTrace-devel wrote:
> On Fri, Apr 19, 2024 at 10:27:11AM +0100, Alan Maguire via DTrace-devel wrote:
> > [resending since appears to have bounced last time]
> >
> > when generating code for print() action we need to identify source
> > of CTF; is it shared CTF, cdefs or ddefs or another module?
> > Use DTRACE_OBJ* values for the first three cases, falling back
> > to using module name pointer as a record argument if these fail.
>
> I am thinking about this... It seems unnecessary to add the conditionals
> in the code since every module *has* a name. So just passing in the module
> name pointer as value would work for all cases anyway. No need to use any
> special cases.
>
> But that brings up a different issue... should we be passing in pointer
> values and expect that they can be retrieved and used as pointers again on
> the consumer end. While that currently would work, it feels a bit fragile.
> Especially if we might have to deal with modules getting unloaded (and that
> might result in the module getting removed from dtrace also - I don't think it
> does right now, but if we start looking towards long running dtrace sessions,
> that might become something to do).
>
> Perhaps we should store the module *name* itself? Slightly less efficient
> but also less fragile?
>
> > Then when consuming records we can use either shared CTF,
> > cdef, ddef or module CTF as appropriate. Nick pointed out that
> > the initial solution will not work for module-defined types.
> >
> > This fixes an issue encountered when using a shared kernel type
> > with alloca()ed memory; DTrace assumed the type was in ddefs
> > and it wasn't so it wasn't displayed. This change fixes that and
> > all tests continue to pass. Also tested that it works with
> > kernel module-defined types now:
> >
> > dtrace: description 'fbt::ieee80211_rx_napi:entry ' matched 1 probe
> > CPU ID FUNCTION:NAME
> > 7 123161 ieee80211_rx_napi:entry 0xffff9f8980fa08e0 = *
> > (struct ieee80211_hw) {
> > .conf = (struct
> > ieee80211_conf) {
> > .listen_interval = (u16)10,
> > .long_frame_max_tx_count =
> > (u8)4,
> > .short_frame_max_tx_count
> > = (u8)7,
> > },
> >
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > ---
> > libdtrace/dt_cg.c | 18 +++++++++++++++++-
> > libdtrace/dt_consume.c | 7 +++++--
> > libdtrace/dt_printf.c | 24 +++++++++++++++++++++---
> > libdtrace/dt_printf.h | 2 +-
> > 4 files changed, 44 insertions(+), 7 deletions(-)
> >
> > diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> > index 27246a40..542fb887 100644
> > --- a/libdtrace/dt_cg.c
> > +++ b/libdtrace/dt_cg.c
> > @@ -20,6 +20,7 @@
> > #include <dt_dctx.h>
> > #include <dt_cg.h>
> > #include <dt_grammar.h>
> > +#include <dt_module.h>
> > #include <dt_parser.h>
> > #include <dt_printf.h>
> > #include <dt_provider.h>
> > @@ -2831,6 +2832,7 @@ dt_cg_act_print(dt_pcb_t *pcb, dt_node_t *dnp,
> > dtrace_actkind_t kind)
> > ctf_file_t *fp = addr->dn_ctfp;
> > ctf_id_t type = addr->dn_type;
> > char n[DT_TYPE_NAMELEN];
> > + const char *ctf_obj;
> > size_t size;
> > type = ctf_type_reference(fp, type);
> > @@ -2842,9 +2844,23 @@ dt_cg_act_print(dt_pcb_t *pcb, dt_node_t *dnp,
> > dtrace_actkind_t kind)
> > "print( ) argument #1 reference has type '%s' with size 0; cannot
> > print( ) it.\n",
> > ctf_type_name(fp, type, n, sizeof(n)));
> > - /* reserve space for addr/type, data/size */
> > + /* reserve space for addr/type, ctf file identification, data/size */
> > addr_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> > sizeof(uint64_t), 8, NULL, type);
> > + /* Use DTRACE_OBJ_* where possible, falling back to module name if
> > + * none of these match.
> > + */
> > + if (fp == dtp->dt_shared_ctf) {
> > + ctf_obj = DTRACE_OBJ_KMODS;
> > + } else if (fp == dtp->dt_cdefs->dm_ctfp) {
> > + ctf_obj = DTRACE_OBJ_CDEFS;
> > + } else if (fp == dtp->dt_ddefs->dm_ctfp) {
> > + ctf_obj = DTRACE_OBJ_DDEFS;
> > + } else {
> > + ctf_obj = dt_module_lookup_by_ctf(dtp, fp)->dm_name;
> > + }
> > + dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> > + sizeof(uint64_t), 8, NULL, (uint64_t)ctf_obj);
> > data_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> > size, 8, NULL, size);
> > diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> > index dec2314b..0d6f2642 100644
> > --- a/libdtrace/dt_consume.c
> > +++ b/libdtrace/dt_consume.c
> > @@ -1987,9 +1987,11 @@ static int
> > dt_print_print(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
> > const caddr_t buf)
> > {
> > - dtrace_recdesc_t *data_rec = rec + 1;
> > + dtrace_recdesc_t *ctf_rec = rec + 1;
> > + dtrace_recdesc_t *data_rec = rec + 2;
> > size_t max_size = dtp->dt_options[DTRACEOPT_PRINTSIZE];
> > size_t size = (size_t)data_rec->dtrd_arg;
> > + const char *ctf_obj = (const char *)ctf_rec->dtrd_arg;
> > uint64_t printaddr;
> > if (size > max_size)
> > @@ -1998,7 +2000,8 @@ dt_print_print(dtrace_hdl_t *dtp, FILE *fp,
> > dtrace_recdesc_t *rec,
> > if (dt_read_scalar(buf, rec, &printaddr) < 0)
> > return dt_set_errno(dtp, EDT_PRINT);
> > - return dt_print_type(dtp, fp, printaddr, (ctf_id_t)rec->dtrd_arg,
> > + return dt_print_type(dtp, fp, printaddr, ctf_obj,
> > + (ctf_id_t)rec->dtrd_arg,
> > (caddr_t)buf + data_rec->dtrd_offset, size);
> > }
> > diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
> > index 50842216..2b5e4e0a 100644
> > --- a/libdtrace/dt_printf.c
> > +++ b/libdtrace/dt_printf.c
> > @@ -14,6 +14,7 @@
> > #include <limits.h>
> > #include <port.h>
> > +#include <dt_module.h>
> > #include <dt_printf.h>
> > #include <dt_string.h>
> > #include <dt_impl.h>
> > @@ -2236,13 +2237,30 @@ err:
> > int
> > dt_print_type(dtrace_hdl_t *dtp, FILE *fp, uint64_t printaddr,
> > - ctf_id_t type, caddr_t data, size_t size)
> > + const char *ctf_obj, ctf_id_t type, caddr_t data, size_t size)
> > {
> > struct dt_visit_arg dva;
> > dva.dv_dtp = dtp;
> > dva.dv_fp = fp;
> > - dva.dv_ctfp = dtp->dt_ddefs->dm_ctfp;
> > +
> > + if (ctf_obj == DTRACE_OBJ_KMODS) {
> > + dva.dv_ctfp = dtp->dt_shared_ctf;
> > + } else if (ctf_obj == DTRACE_OBJ_CDEFS) {
> > + dva.dv_ctfp = dtp->dt_cdefs->dm_ctfp;
> > + } else if (ctf_obj == DTRACE_OBJ_DDEFS) {
> > + dva.dv_ctfp = dtp->dt_ddefs->dm_ctfp;
> > + } else {
> > + struct dt_module *dm = dt_module_lookup_by_name(dtp, ctf_obj);
> > +
> > + if (dm)
> > + dva.dv_ctfp = dt_module_getctf(dtp, dm);
> > + if (!dm || !dva.dv_ctfp) {
> > + dt_dprintf("error retrieving CTF for print() action\n");
> > + return dt_set_errno(dtp, EDT_CTF);
> > + }
> > + }
> > +
> > dva.dv_data = data;
> > dva.dv_size = size;
> > dva.dv_last_depth = 0;
> > @@ -2260,5 +2278,5 @@ dt_print_type(dtrace_hdl_t *dtp, FILE *fp,
> > uint64_t printaddr,
> > if (dt_print_close_parens(&dva, 0) < 0)
> > return -1;
> > - return 2;
> > + return 3;
> > }
> > diff --git a/libdtrace/dt_printf.h b/libdtrace/dt_printf.h
> > index 9771c4a8..b72ff55b 100644
> > --- a/libdtrace/dt_printf.h
> > +++ b/libdtrace/dt_printf.h
> > @@ -108,7 +108,7 @@ extern int dt_print_ustack(dtrace_hdl_t *, FILE *,
> > const char *, caddr_t, uint64_t);
> > extern int dt_print_mod(dtrace_hdl_t *, FILE *, const char *, caddr_t);
> > extern int dt_print_umod(dtrace_hdl_t *, FILE *, const char *, caddr_t);
> > -extern int dt_print_type(dtrace_hdl_t *, FILE *, uint64_t, ctf_id_t,
> > +extern int dt_print_type(dtrace_hdl_t *, FILE *, uint64_t, const char
> > *, ctf_id_t,
> > caddr_t, size_t);
> > #ifdef __cplusplus
> > --
> > 2.39.3
> >
> >
> > _______________________________________________
> > DTrace-devel mailing list
> > DTrace-devel@oss.oracle.com
> > https://oss.oracle.com/mailman/listinfo/dtrace-devel
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
^ permalink raw reply [flat|nested] only message in thread