<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">&lt;eugene.loh@oracle.com&gt;</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-&gt;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.&nbsp; 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.&nbsp; 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.&nbsp;
    (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.)&nbsp; For stack([n]) calls, a lower maxframes
    value does us no good.&nbsp; So, really, the option you are proposing
    should be named in some way that specifies that it's related to
    stackdepth.&nbsp; Then, we would unnecessarily be imposing the same limit
    on stack().&nbsp; 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 &quot;option&quot; for a future patch once we have a
    better sense of what's worth doing.<br>
    <br>
    [Not sure that's clear.&nbsp; The point is that the only &quot;wasting space&quot;
    is just for stackdepth.&nbsp; There is no such issue for stack().&nbsp; 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>