[DTrace-devel] restoring how DTrace adds probes

[Kris Van Hees]

Eugene,

Thank you for the analysis and looking into a mechanism for creating new
probes (especially for profile-* and tick-* probes) in DTrace v2.  I added
some inline Comments below.

On Mon, Jun 08, 2020 at 12:25:06PM -0700, Eugene Loh wrote:
> We should change DTrace's loading of probes to respond to which probes 
> are requested by the user.  That is, we are currently populating probes 
> in a fixed manner without regard for which probe descriptions were 
> specified by the user, while legacy DTrace would load probes as 
> requested by the user.  Restoring the legacy behavior and taking user 
> requests in account is important for some probes -- like profile-* and 
> tick-* -- that do not exist until the user requests them.

I have to disagree with your summary, unfortuantely.  From the perspective of
userspace you are correct: legacy DTrace would discover probes on an as-needed
basis as it resolved probe descriptions,  However, the kernel level DTrace
component maintained a list of all existing probes (used to resolve requests
from the userspace component for matching probes against probe descriptions).

In DTrace v2 there is no DTrace specific kernel component, so we no longer have
a list of probes to match against.  DTrace v2 must provide for the full DTrace
functionality in userspace alone.

> BACKGROUND.
> 
> Note that in the dtrace command-line tool, there are five passes through 
> the command-line arguments.  Important events are:
>    - the call to dtrace_open()
>    - the second pass over arguments, collecting program specifications
>    - the fourth pass, compiling these specifications
> 
> In legacy DTrace() (e.g., the master branch), probes are looked up -- or 
> created if necessary -- during that fourth-pass compilation.  That is, 
> we call
> dt_compile_one_clause(),
> which calls dt_setcontext() to look for a "representative probe",
> which calls dt_probe_info(),
> which calls dtrace_probe_iter(),
> which dt_ioctl(dtp, DTRACEIOC_PROBEMATCH, &pd) to look up probes.
> 
> Then, in the kernel, dtrace_ioctl() has:
>      /*
>       * Before we attempt to match this probe, we want to give
>       * all providers the opportunity to provide it.
>       */
>      dtrace_probe_provide(&desc, NULL);
> 
> That function loops over providers.  Each provider can provide probes to 
> satisfy the specified probe description.

You are missing an important step...  legacy DTrace would open the
/dev/dtrace/dtrace pseudo-device file to obtain a file descriptor to issue
ioctl requests on.  That open request got handled by the dtrace kernel module,
and one of the first things it does is:

	dtrace_probe_provide(NULL, NULL);

which means that it requests all loaded providers to register their probes with
the DTrace framework in the kernel.  Once this call completes, the kernel has
a complete list of available probes to be used when performing probe match
requests.

> In contrast, in the most recent implementation of DTrace (e.g., the 
> 2.0-branch-dev branch), near the end of dtrace_open() -- strictly 
> speaking, dt_vopen() -- we add:
>      /*
>       * Initialize the collection of probes that is made available by the
>       * known providers.
>       */
>      dt_probe_init(dtp);
>      for (i = 0; i < ARRAY_SIZE(dt_providers); i++)
>                  dt_providers[i]->populate(dtp);
> 
> That is, we loop over providers to populate probes.  This is done before 
> looking at any of the probe descriptions in command-line arguments or 
> specified scripts.  The probes are provided from set lists, such as 
> predefined dtrace probes (e.g., BEGIN and END) or probes found in lists 
> in the tracefs file system.

As I describe in my comment above, this is what legacy DTrace does.  The new
implementation at the userspace level is functionally equivalent to what the
kernel was doing.

> Later, for example in dt_probe_iter(), the list of probes having been 
> defined, we simply look up whether a probe is known or not. We do not 
> currently allow a user to specify a probe that we did not already know.
> 
> I don't think I've thoroughly captured how the kernel handles probe 
> queries in legacy DTrace;  the main point is simply that DTrace used to 
> provide probes "on demand" while we now pre-define probe lists.

Well, no, DTrace used to provide pre-defined probes and also had a mechanism
to allow providers to create probes on demand.  DTrace v2 does not have that
mechanism yet (and I believe you were looking at adding that functionality
to the provider api).

> PROPOSAL.
> 
> Instead of the current "populate()" approach, in which each provider 
> populates the list of probes without knowledge of the user's probe 
> descriptions, let us revert to legacy DTrace's on-demand "provide()" 
> approach, in which we track probes exclusively in response to 
> user-specified probe descriptions. Specifically,

As explained above, the behaviour your want to revert to isn't the behaviour of
legacy DTrace at all.

> *)  In dt_vopen(), remove the new code that loops over providers calling 
> their populate() functions.

No, that is what legacy DTrace was doing as well.

> *)  In dt_probe_iter(), allow providers to provide a probe.

We need to do the equivalent of the kernel implementation in legacy DTrace when
a probe description was passed to a provider to see if it can provide the
requested probe(s).  I expect we want to add a function to the provider ops
struct to facilitate providing unknown probes.

> DISCUSSION.
> 
> *)  We could use both mechanisms.  That is, during dtrace_open(), we can 
> use the "populate()" model to populate probes without regards to 
> user-specified probe descriptions.  Then later, when compiling probe 
> descriptions, add probes as necessary.  But there is no value in having 
> two mechanisms, and there is no sense in adding possibly tens of 
> thousands of probes even in the relatively common case where the user 
> specifies fewer than a dozen.

We may move towards a more selective process of what probes we keep track of
as an optimization but only after we implement the legacy behaviour correctly.
Keep in mind that probe matching is done for every probe specification in a D
script, so if you do not pre-populate the probe list (and lookup hashtables)
you may end up reading and parsing the kernel lists multiple times.

> *)  We should clean up these two functions:
>          - dtrace_probe_iter()
>          -     dt_probe_iter()
> Once again, we should revert somewhat to the legacy implementation.  
> That is, dt_probe_iter() used to be a static function that was used 
> simply as a callback within dtrace_probe_iter().  In the current 
> version, dt_probe_iter() is no longer a static function.  On the other 
> hand, it is still used only by dtrace_probe_iter().  And 
> dtrace_probe_iter() has become simply a wrapper for dt_probe_iter().  
> Therefore:
>      - move probe iteration from dt_probe_iter() into dtrace_probe_iter()
>      - if dt_probe_iter() is still needed, make it static once again

That code is still in need of further refining as we move towards supporting
wildcard probe descriptions in full.  So, we will need to evaluate the
interacton between dtrace_probe_iter and dt_probe_iter().  Note that the
dtrace_probe_iter() is the function that is exported as part of the libdtrace
API whereas dt_probe_iter() provides the implementation that is planned to be
called from other parts of libdtrace itself,

> *)  DTrace has the IMHO peculiar practice of listing probes multiple 
> times.  Consider:
>      dtrace -n END -l -n BEGIN -n BEGIN
>         ID   PROVIDER    MODULE    FUNCTION NAME
>          2     dtrace                       END
>          1     dtrace                       BEGIN
>          1     dtrace                       BEGIN
> This behavior is seen in both legacy and the current DTrace. Leaving 
> this behavior is fine.

The -l option is an operation option, that is an option that specifies which
action is to be taken by dtrace (listing probes used in clauses).  In this case
you specify three clauses so you get the list of probes used in each clause.
There is no code to remove duplicates because probe descriptions are processed
as independent units.

> PROFILE PROVIDER.
> 
> While many providers can benefit from this proposal, it was motivated by 
> the immediate desire to add a profile provider.  The following 
> discussion is in that specific context.

The proposed change is not necessary for adding functionality to create
probes dynamically.

> *)  While users can specify their own profile probes, the provider 
> should nonetheless provide some suggestive probes, following legacy 
> DTrace's suit.  E.g., from the legacy kernel file dtrace/profile_dev.c:
>      profile_rates[] = { 97, 199, 499, 997, 1999, 4001, 4999, ... };
>      profile_ticks[] = { 1, 10, 100, 500, 1000, 5000, ...};

Yes.  Those need to be pre-populated.

> *)  A user probe description such as "profile:::" would clearly refer to 
> all existing profile probes rather than to all conceivable profile probes.

Yes, which is also the legacy behaviour.

> *)  Legacy DTrace added profile probes persistently.  That is, if a user 
> specified profile-1234, that probe would persist -- even to other DTrace 
> invocations -- until the profile kernel module were unloaded.  The 
> current implementation need not mimic such persistent behavior.

Whether dynamically created profile and tick probes are persistent or not is
indeed not important other than the fact that we need to ensure that if probes
are not task-specific they cannot be destroyed until all instances are done
using them.