<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p>On 6/9/21 1:21 AM, Eugene Loh wrote:<br>
</p>
<blockquote type="cite" cite="mid:545c07eb-d17e-544e-1db4-753968ee3812@oracle.com">
<p>On 6/9/21 12:43 AM, Kris Van Hees wrote:<br>
</p>
<blockquote type="cite" cite="mid:20210609044309.GS10024@oracle.com">
<pre class="moz-quote-pre" wrap=""> Fri, May 28, 2021 at 02:35:09PM -0400, <a class="moz-txt-link-abbreviated" href="mailto:eugene.loh@oracle.com" moz-do-not-send="true">eugene.loh@oracle.com</a> wrote:
</pre>
<blockquote type="cite" style="color: #007cff;">
<pre class="moz-quote-pre" wrap="">From: Eugene Loh <a class="moz-txt-link-rfc2396E" href="mailto:eugene.loh@oracle.com" moz-do-not-send="true"><eugene.loh@oracle.com></a>
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.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">It seems that the max frames setting should perhaps be an option, just
like various other tunables?</pre>
</blockquote>
<br>
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.<br>
<br>
<blockquote type="cite" cite="mid:20210609044309.GS10024@oracle.com">
<pre class="moz-quote-pre" wrap="">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).</pre>
</blockquote>
</blockquote>
<br>
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.<br>
<br>
How about we leave this "option" for a future patch once we have a
better sense of what's worth doing.<br>
<br>
[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.]<br>
<blockquote type="cite" cite="mid:545c07eb-d17e-544e-1db4-753968ee3812@oracle.com">
<blockquote type="cite" cite="mid:20210609044309.GS10024@oracle.com">
<pre class="moz-quote-pre" wrap="">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?</pre>
</blockquote>
</blockquote>
</body>
</html>