[DTrace-devel] [PATCH v2] print() action: identify ctf object used for print

Alan Maguire alan.maguire at oracle.com
Thu Apr 25 13:17:36 UTC 2024


On 24/04/2024 18:19, Kris Van Hees 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?
>

What about having a monotonic module id assocaited with each module on
module creation; then we could store the module's id at cg time. Later
when we consume the print() records we could lookup the module and
associated BTF via that id? Would mean we'd just need to store a 64-bit
value, and would be less fragile than storing a pointer. Thanks!

Alan


>> 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 at 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 at oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/dtrace-devel



More information about the DTrace-devel mailing list