Re: [PATCH 20/38] Add a hook for a provider-specific "update" function

[Eugene Loh <eugene.loh@oracle.com>]

The plane has barely taken off the ground and already you expect me to 
have completed the aircraft design???  :^)  (I was hoping the magic 
letters WIP in the commit message would have absolved me of 
responsibility here.)

Would you be willing to proceed with reviews of the other patches and 
then we get back to this patch?  There is a clearly provider-specific 
motivation for this function, but the style is to make these functions 
as provider-agnostic as possible.  Maybe we can defer some of the 
questions this patch raises.  Like, how often will the function be 
called, what triggers that call, will the call be USDT-specific or will 
it check all providers, etc. For now, I only have USDT-specific answers, 
but we need to guess what other considerations we expect to have to 
accommodate.

On 7/19/24 16:31, Kris Van Hees wrote:
> This patch surely should be part of another one that actually provides some
> code to use the new callback.  The implementation of the callback in any
> particular provider could remain in its own patch after that if you like,
> but adding this without the code that actually calls it seems less useful.
> If anything, the commit message itself makes it so generic sounding that one
> is likely to have no idea what this is meant to be used for, where it would
> be called from, why, and what the expected result ought to be.
>
> So, combine with the code that uses it please.  Which I will review next :)
>
> On Thu, Jun 27, 2024 at 01:38:46AM -0400, eugene.loh@oracle.com wrote:
>> From: Eugene Loh <eugene.loh@oracle.com>
>>
>> For up-coming USDT-probe support, we need to update a BPF map
>> -- at least when the dtrace session starts but possibly also later
>> to support systemwide USDT tracing for processes that may start up
>> later.
>>
>> One way to do this is with a USDT-specific update function.
>>
>> For now, let's add a hook for providers to have provider-specific
>> update functions.  User space can either call
>>
>>      for (i = 0; i < ARRAY_SIZE(dt_providers); i++) {
>>          if (dt_providers[i]->update)
>>              dt_providers[i]->update(...);
>>      }
>>
>> any time it likes.  Or it can call dt_usdt.update(...).
>>
>> This is for WIP.  A different approach can be adopted later instead.
>>
>> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
>> ---
>>   libdtrace/dt_provider.h | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/libdtrace/dt_provider.h b/libdtrace/dt_provider.h
>> index b1b1b1b8..71b5a3c4 100644
>> --- a/libdtrace/dt_provider.h
>> +++ b/libdtrace/dt_provider.h
>> @@ -71,6 +71,8 @@ typedef struct dt_provimpl {
>>   			      void *datap);
>>   	void (*destroy)(dtrace_hdl_t *dtp,	/* free provider data */
>>   			      void *datap);
>> +	void (*update)(dtrace_hdl_t *dtp,	/* update provider-specific info */
>> +			      void *datap);
>>   } dt_provimpl_t;
>>   
>>   /* list dt_dtrace first */
>> -- 
>> 2.18.4
>>