Forw: [Bug translator/1276] support more timer varieties

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Forw: [Bug translator/1276] support more timer varieties

Frank Ch. Eigler
----- Forwarded message from "Stone, Joshua I" <[hidden email]> -----
So, pt_regs is fine now in timer.profile.  But it would be even more
useful as a profiler if other context information was also available,
like pid, tid, execname, etc.  That information can be gotten through
current->..., but our context tapsets disables this, like so:

  function pid:long () %{
      if (unlikely(in_interrupt()))
          THIS->__retvalue = 0;
      else
          THIS->__retvalue = current->tgid;
  %}

timer.profile is always in interrupt, so current->tgid is not accessed.
I tried this without the in_interrupt check, and the pids I got looked
fine.  So, at least empirically, it seems it's fine to access current
within an interrupt.

The reason you might NOT want to read from current in an interrupt is
that it may be misleading to the user if they're not expecting this.
e.g. the user may have a function probe that gets triggered during an
interrupt, but it would be incorrect to attribute that to the task in
current.

So I see two quick ways to fix this:

1) remove the in_interrupt check from the existing taps, and perhaps put
some kind of warning in the user docs.
2) create an alternate set of context taps (perhaps _pid) that doesn't
have the in_interrupt check.  This may create additional confusion for
the user though, and any other tapsets that call pid(), tid(), etc.
would also have to be duplicated.

I think the best solution would be if we had some sort of global flag
that indicated that we're in a profiling interrupt.  That way we could
have something like this:

  function pid:long () %{
      if (unlikely(in_interrupt() && !_stp_in_profiling_interrupt))
          THIS->__retvalue = 0;
      else
          THIS->__retvalue = current->tgid;
  %}

Comments?

Josh
---- End forwarded message -----