[DTrace-devel] [PATCH v4 1/3] print() action: identify ctf object used for print
Alan Maguire
alan.maguire at oracle.com
Thu Aug 1 21:30:13 UTC 2024
On 01/08/2024 17:50, Kris Van Hees wrote:
> Small question below...
>
> On Mon, Jul 08, 2024 at 03:54:02PM +0100, Alan Maguire wrote:
>> when generating code for print() action we need to identify source
>> of CTF; is it shared CTF, cdefs or ddefs or another module?
>> Rework the print() action to be a printf()like function under the
>> hood using the format specifier as module name. We can then
>> do away with dt_print_print(), calling dt_print_type() directly
>> from consume.
>>
>> Suggested-by: Kris Van Hees <kris.van.hees at oracle.com>
>> Signed-off-by: Alan Maguire <alan.maguire at oracle.com>
>> ---
>> libdtrace/dt_cg.c | 8 +++++++-
>> libdtrace/dt_consume.c | 28 +++-------------------------
>> libdtrace/dt_impl.h | 1 +
>> libdtrace/dt_printf.c | 28 ++++++++++++++++++++++++----
>> libdtrace/dt_printf.h | 6 ++++--
>> 5 files changed, 39 insertions(+), 32 deletions(-)
>>
>> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>> index a1c24e37..2eb2df9c 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>
>> @@ -2838,8 +2839,11 @@ 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];
>> + dt_module_t *dmp;
>> + dt_pfargv_t *pfp;
>> size_t size;
>>
>> + dmp = dt_module_lookup_by_ctf(dtp, fp);
>> type = ctf_type_reference(fp, type);
>> if (type == CTF_ERR)
>> longjmp(yypcb->pcb_jmpbuf, EDT_CTF);
>> @@ -2849,9 +2853,11 @@ 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)));
>>
>> + pfp = dt_printf_create(dtp, dmp->dm_name);
>> +
>> /* reserve space for addr/type, data/size */
>> addr_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
>> - sizeof(uint64_t), 8, NULL, type);
>> + sizeof(uint64_t), 8, pfp, type);
>> data_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
>> size, 8, NULL, size);
>
> Why pass 'size' as record argument when you are already passing it as the true
> size of the record? That's unnecessary. We can clearly get the size of the
> record from the 2nd record's dtrd_size.
>
yep, good point. I've sent a v5 which doesn't pass it as an arg and uses
record size below as suggested. Thanks!
Alan
>> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
>> index dec2314b..e6880703 100644
>> --- a/libdtrace/dt_consume.c
>> +++ b/libdtrace/dt_consume.c
>> @@ -507,7 +507,7 @@ dt_nullrec()
>> return DTRACE_CONSUME_NEXT;
>> }
>>
>> -static int
>> +int
>> dt_read_scalar(caddr_t addr, const dtrace_recdesc_t *rec, uint64_t *valp)
>> {
>> addr += rec->dtrd_offset;
>> @@ -1983,25 +1983,6 @@ dt_print_trace(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
>> return dt_print_rawbytes(dtp, fp, data, rec->dtrd_size);
>> }
>>
>> -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;
>> - size_t max_size = dtp->dt_options[DTRACEOPT_PRINTSIZE];
>> - size_t size = (size_t)data_rec->dtrd_arg;
>> - uint64_t printaddr;
>> -
>> - if (size > max_size)
>> - size = max_size;
>> -
>> - 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,
>> - (caddr_t)buf + data_rec->dtrd_offset, size);
>> -}
>> -
>> /*
>> * The lifecycle of speculation buffers is as follows:
>> *
>> @@ -2562,11 +2543,8 @@ dt_consume_one_probe(dtrace_hdl_t *dtp, FILE *fp, char *data, uint32_t size,
>> func = dtrace_freopen;
>> break;
>> case DTRACEACT_PRINT:
>> - n = dt_print_print(dtp, fp, rec, data);
>> - if (n < 0)
>> - return -1;
>> - i += n - 1;
>> - continue;
>> + func = dt_print_type;
>> + break;
>> default:
>> break;
>> }
>> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
>> index 01313ff3..88b31ce0 100644
>> --- a/libdtrace/dt_impl.h
>> +++ b/libdtrace/dt_impl.h
>> @@ -789,6 +789,7 @@ extern int dt_buffered_flush(dtrace_hdl_t *, dtrace_probedata_t *,
>> const dtrace_recdesc_t *, const dtrace_aggdata_t *, uint32_t flags);
>> extern void dt_buffered_disable(dtrace_hdl_t *);
>> extern void dt_buffered_destroy(dtrace_hdl_t *);
>> +extern int dt_read_scalar(caddr_t, const dtrace_recdesc_t *, uint64_t *);
>>
>> extern uint64_t dt_stddev(uint64_t *, uint64_t);
>>
>> diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
>> index 50842216..d794dd47 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>
>> @@ -2235,15 +2236,34 @@ err:
>> }
>>
>> int
>> -dt_print_type(dtrace_hdl_t *dtp, FILE *fp, uint64_t printaddr,
>> - ctf_id_t type, caddr_t data, size_t size)
>> +dt_print_type(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
>> + const dtrace_probedata_t *data, const dtrace_recdesc_t *recs,
>> + uint_t nrecs, const void *buf, size_t len)
>> {
>> + const char *modname = ((dt_pfargv_t *)fmtdata)->pfv_format;
>> + size_t max_size = dtp->dt_options[DTRACEOPT_PRINTSIZE];
>> + ctf_id_t type = (ctf_id_t)recs->dtrd_arg;
>> + const dtrace_recdesc_t *data_rec = recs + 1;
>> + size_t size = (size_t)data_rec->dtrd_arg;
>
> Use data_rec->dtrd_size. No need to use dtrd_arg with a cast.
>
>> + caddr_t addr_data = (caddr_t)buf;
>> struct dt_visit_arg dva;
>> + uint64_t printaddr;
>> + dt_module_t *dmp;
>>
>> + if (size > max_size)
>> + size = max_size;
>> +
>> + if (dt_read_scalar(addr_data, recs, &printaddr) < 0)
>> + return dt_set_errno(dtp, EDT_PRINT);
>> +
>> + dmp = dt_module_lookup_by_name(dtp, modname);
>> + if (!dmp) {
>> + return dt_set_errno(dtp, EDT_PRINT);
>> + }
>> dva.dv_dtp = dtp;
>> dva.dv_fp = fp;
>> - dva.dv_ctfp = dtp->dt_ddefs->dm_ctfp;
>> - dva.dv_data = data;
>> + dva.dv_ctfp = dmp->dm_ctfp;
>> + dva.dv_data = addr_data + data_rec->dtrd_offset;
>> dva.dv_size = size;
>> dva.dv_last_depth = 0;
>> dva.dv_startindent = DT_PRINT_STARTINDENT;
>> diff --git a/libdtrace/dt_printf.h b/libdtrace/dt_printf.h
>> index 9771c4a8..4f4386a5 100644
>> --- a/libdtrace/dt_printf.h
>> +++ b/libdtrace/dt_printf.h
>> @@ -108,8 +108,10 @@ 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,
>> - caddr_t, size_t);
>> +extern int dt_print_type(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
>> + const dtrace_probedata_t *data,
>> + const dtrace_recdesc_t *recs,
>> + uint_t nrecs, const void *buf, size_t len);
>>
>> #ifdef __cplusplus
>> }
>> --
>> 2.43.5
More information about the DTrace-devel
mailing list