[DTrace-devel] [PATCH 05/12] Add support for the stack() action

[Eugene Loh]

On 6/9/21 1:21 AM, Eugene Loh wrote:

> On 6/9/21 12:43 AM, Kris Van Hees wrote:
>
>>   Fri, May 28, 2021 at 02:35:09PM -0400,eugene.loh at oracle.com  wrote:
>>> From: Eugene Loh<eugene.loh at oracle.com>
>>>
>>> Implement the stack() action using the bpf_get_stack() helper
>>> function.  This implementation most closely resembles the legacy
>>> DTrace implementation.  Someday it may make sense to switch over
>>> to the bpf_get_stackid() instead, which would allow consolidation
>>> of like stacks.
>>>
>>> The max stack size can be controlled by the kernel parameter
>>> perf_event_max_stack, and we would like to allow users to increase
>>> that limit.  We will eventually need to know this limit in various
>>> places.  So, add a dtp->dt_maxframes value.
>> It seems that the max frames setting should perhaps be an option, just
>> like various other tunables?
>
> I can imagine that, but that would be a new option and I would see it 
> being a separate patch.  If you really want it in this patch, just let 
> me know.
>
>> I could see a potential use for setting the
>> maxframes value to be less than the system max (e.g. to avoid wasting space
>> in buffers because I know I won't need the max anyway).

I wanted to add another comment here.  The only case where a lower value 
would be of interest is for [u]stackdepth support, where we need to set 
aside some room for a stack that doesn't interest us. (We only care 
about the return value of bpf_get_stack(), but for the stackdepth to be 
reflected in that return value we need the stack buffer to be large 
enough.)  For stack([n]) calls, a lower maxframes value does us no 
good.  So, really, the option you are proposing should be named in some 
way that specifies that it's related to stackdepth.  Then, we would 
unnecessarily be imposing the same limit on stack().  Or, we don't need 
to impose the stackdepth limit on stack(), but then we still need to 
know the kernel limit, which was what I was trying to do.

How about we leave this "option" for a future patch once we have a 
better sense of what's worth doing.

[Not sure that's clear.  The point is that the only "wasting space" is 
just for stackdepth.  There is no such issue for stack().  For stack(), 
the only issue is whether stack(n) is requesting more than the kernel 
will permit.]
>> So, the default
>> would be the perf_event_max_stack value, but it would be something the user
>> can override with a pragma or -x option.  Then you an access the value just
>> like any other option, and you do not need to add another member to the
>> dtrace_hdl_t struct.
>>
>> Since there is a stackframes option, perhaps use maxstackframes for this?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/dtrace-devel/attachments/20210609/97bbef66/attachment-0001.html