[DTrace-devel] [PATCH v2 15/19] Ignore clauses: some clauses are impossible regardless of uprp

[Eugene Loh]

On 10/23/24 16:28, Kris Van Hees wrote:
> On Tue, Sep 24, 2024 at 04:25:54PM -0400, eugene.loh at oracle.com wrote:
>> From: Eugene Loh <eugene.loh at oracle.com>
>>
>> In ignore_clauses, for an underlying probe uprp, we try to
>> decide if we can safely ignore clause n.
>>
>> Meanwhile, for some clauses, the probe description tells us the
>> clause will not be called for any USDT probe, regardless of the
>> underlying probe.  For example, "syscall::write:" can safely be
>> ignored, for all uprp.
>>
>> Add a dtsd_usdt variable to each statement to track status:
>>
>>      USDT_FLAG_UNINITIALIZED  not yet initialized
>>
>>      USDT_FLAG_POSSIBLE       clause could possibly be called
>>                               for some USDT probe
>>
>>      USDT_FLAG_IGNORE         clause can safely be ignored for
>>                               all USDT probes
> I think it would be better to use the dtsd_clauseflags member for this.
> You can add dt_stmt_set_flag() and dt_stmt_test_flag() (or similar names)
> to set and test for specific bits in the dtsd_clauseflags member using
> DT_CLSFLAG_* constants.  You should be OK with just 2, one to indicate
> POSSIBLE and one to indicate IGNORE (neither being set obviously meansnot
> yet initialized).
>
> I don't really know what symbol names would be best... perhaps for now use
> DT_CLSFLAG_USDT_INCLUDE and DT_CLSFLAG_USDT_EXCLUDE?
>
> Main thing I would like to accomplish here is simply to access flags on the
> statements through functions rather than accessing them directly.

I'd like to push back here.

First of all, the patch exposes only dtsd_usdt at the dtrace.h level.  
That variable can hide whatever it is that USDT wants to do with it.  
Your proposal would expose at least USDT_INCLUDE/EXCLUDE (or 
POSSIBLE/IGNORE or whatever we want to call them) in dtrace.h, and I do 
not know if future USDT would have even more stuff it wants to play 
with, meaning even more changes outside of the provider.

Second, you say the main point is to access flags through functions 
rather than directly, but to date we've been accessing flags directly.  
The set/test functions would be new.  If we want to stop the direct 
accesses, that would seem to be a different patch.

Thoughts?