Re: [PATCH 1/2] fbt: Populate just once

On 1/15/26 16:35, Kris Van Hees wrote:

> On Tue, Jan 13, 2026 at 04:42:04PM -0500, [email protected] wrote:
>> From: Eugene Loh <[email protected]>
>>
>> In commit 0b7c5a632 ("fbt, rawfbt: consolidate code to avoid duplication"),
>> the populate() functions for fbt and rawfbt were combined, populating both
>> providers, but the function was still called twice.  That is, dt_open.c
>> tries to insert each provider twice.
> Where do you see evidence that the populate function is being called twice?
> Only the providers listed in the static initial list have populate() called,
> and while &dt_fbt is in that list, &dt_rawfbt is not.  Also, adding debugging
> output to populate() in dt_prov_fbt.c (or enabling DTRACE_DEBUG) shows that
> it is only called once.

I agree with you.  Thanks.  I'll resubmit the patch, keeping only the 
"harmless safety net" and typo correction, along with an appropriately 
reworded subject line.

>> One solution would be to have a different populate() function for each
>> provider.
>>
>> Here, we employ another solution:  in dt_provider_create(), check to see
>> if a specified provider has already been inserted.
> This change is not needed but it is also harmless and can avoid future problems
> so I would not object having this as a safety net.
>
>> This also requires a corresponding change in dt_provider_lookup(), so
>> that it will work even if no providers have yet been inserted.
>>
>> Also, fix a minor comment in the fbt provider.
> Thanks for this fix.
>
>> Signed-off-by: Eugene Loh <[email protected]>
>> ---
>>   libdtrace/dt_prov_fbt.c | 2 +-
>>   libdtrace/dt_provider.c | 6 ++++++
>>   2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
>> index bbe44a842..3feac56ea 100644
>> --- a/libdtrace/dt_prov_fbt.c
>> +++ b/libdtrace/dt_prov_fbt.c
>> @@ -228,7 +228,7 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
>>   		if (!dt_symbol_traceable(sym))
>>   			continue;
>>   
>> -		/* Function name cannot be synthetic and must match. */
>> +		/* Function name cannot be synthetic (unless rawfbt) and must match. */
>>   		fun = dt_symbol_name(sym);
>>   		if ((!rawfbt && strchr(fun, '.')) || !dt_gmatch(fun, pdp->fun))
>>   			continue;
>> diff --git a/libdtrace/dt_provider.c b/libdtrace/dt_provider.c
>> index 848fdc132..9d75225fa 100644
>> --- a/libdtrace/dt_provider.c
>> +++ b/libdtrace/dt_provider.c
>> @@ -114,6 +114,8 @@ dt_provider_lookup(dtrace_hdl_t *dtp, const char *name)
>>   		return NULL;
>>   
>>   	strcpy(tmpl.desc.dtvd_name, name);
>> +	if (dtp->dt_provs == NULL)
>> +		return NULL;
>>   	return dt_htab_lookup(dtp->dt_provs, &tmpl);
>>   }
>>   
>> @@ -124,6 +126,10 @@ dt_provider_create(dtrace_hdl_t *dtp, const char *name,
>>   {
>>   	dt_provider_t *pvp;
>>   
>> +	pvp = dt_provider_lookup(dtp, name);
>> +	if (pvp)
>> +		return pvp;
>> +
>>   	if ((pvp = dt_zalloc(dtp, sizeof(dt_provider_t))) == NULL)
>>   		goto nomem;
>>   
>> -- 
>> 2.47.3
>>