public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* systemtap bug #6500 fallout
@ 2008-05-12 23:32 David Smith
  2008-05-12 23:41 ` Frank Ch. Eigler
  0 siblings, 1 reply; 5+ messages in thread
From: David Smith @ 2008-05-12 23:32 UTC (permalink / raw)
  To: Frank Ch. Eigler, Ananth Mavinakayanahalli; +Cc: Systemtap List

Frank/Ananth,

I've fixed bug #6500 (utrace support doesn't handle multithread apps
properly), but caused some interesting fallout.

Originally, here's how the utrace probe layer worked with the task
finder layer.  The task finder attached a utrace engine to all
non-kernel threads.  A utrace probe gets registered with the task finder
layer by telling it which threads it finds "interesting" (by specifying
either the pid or pathname).  When the task finder layer found an
interesting thread, it called a callback function that the utrace probe
layer provides.  Its signature looks like this:

typedef int (*stap_task_finder_callback)(struct task_struct *tsk,

                                         int register_p,

                                         struct stap_task_finder_target
*tgt);

If register_p is 1, the utrace probe layer attaches an engine with any
other events it is interested in (like syscall events) to the thread.
If the user was interested in an exec event, the probe handler gets
called in the callback (if you add an utrace engine that wants to handle
exec while you are in an exec event, it won't get called).

If register_p was 0, the utrace probe layer is notified that the thread
is dying and cleans up.  It also calls any death probe handler.

This system worked well for traditional fork/exec style programs (except
I now realize you'd miss syscalls executed in the child process between
the fork and the exec).


So, to fix this problem for multi-threaded programs, the callback gets
called (with register_p=1) for clone *and* exec events on "interesting"
threads.  This seems to work well for multi-threaded programs.  However,
for traditional fork/exec style programs, this has caused a behavior
change (besides that any exec probes are probably getting called too
often).  Let's say you have the following script:

  probe process("/bin/bash").syscall {
    printf("/bin/bash(%d) syscall\n", tid())
  }
  probe process("/bin/ls").syscall {
    printf("/bin/ls(%d) syscall\n", tid())
  }

Then, from a bash session run ls.  Originally the output would look
something like this:

1: the syscalls for bash (from pid X - note that you wouldn't see
syscalls in the newly forked bash)
2: the syscalls for the exec'ed ls (from new pid Y)
3: the syscalls for bash (from original pid X)

After the bug #6500 fix, you'll see something like this:

1: the syscalls for bash (from pid X)
2: after the fork, syscalls from the new bash (from new pid Y)
3: after the exec, the syscalls for ls (from new pid Y) reported for
both bash *and* ls
4: the syscalls for bash (from original pid X)

Number 3 above doesn't seem correct to me.  Once the exec happens, the
pid is no longer 'bash'.  But, because the bash utrace engine wasn't
detached from that pid, the events still get reported.


So, my tentative plan is to have the task finder layer call the callback
 (potentially) twice when an exec happens - once to unregister the "old"
path and once to register the "new" path.

Does this seem reasonable?

Thanks.

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: systemtap bug #6500 fallout
  2008-05-12 23:32 systemtap bug #6500 fallout David Smith
@ 2008-05-12 23:41 ` Frank Ch. Eigler
  2008-05-13  0:00   ` Frank Ch. Eigler
  0 siblings, 1 reply; 5+ messages in thread
From: Frank Ch. Eigler @ 2008-05-12 23:41 UTC (permalink / raw)
  To: David Smith; +Cc: Ananth Mavinakayanahalli, Systemtap List

David Smith <dsmith@redhat.com> writes:

> [...]
> So, my tentative plan is to have the task finder layer call the callback
>  (potentially) twice when an exec happens - once to unregister the "old"
> path and once to register the "new" path.

Yeah, that makes sense.  Further, since an "exec" is a process-wide
event, so I'd expect death notifications for all threads other than
the caller.  An "exit" is also process-wide; I presume the per-thread
death notifications work there already.  I smell the need for a good
pass-5 pthread test case.

- FChE

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: systemtap bug #6500 fallout
  2008-05-12 23:41 ` Frank Ch. Eigler
@ 2008-05-13  0:00   ` Frank Ch. Eigler
  2008-05-13 10:43     ` Ananth N Mavinakayanahalli
  0 siblings, 1 reply; 5+ messages in thread
From: Frank Ch. Eigler @ 2008-05-13  0:00 UTC (permalink / raw)
  To: David Smith; +Cc: Ananth Mavinakayanahalli, Systemtap List


I wrote:

>> [...]
>> So, my tentative plan is to have the task finder layer call the callback
>>  (potentially) twice when an exec happens - once to unregister the "old"
>> path and once to register the "new" path.
>
> Yeah, that makes sense.  Further, since an "exec" is a process-wide
> event, so I'd expect death notifications for all threads other than
> the caller.  An "exit" is also process-wide; I presume the per-thread
> death notifications work there already.  [...]

By the way, this does not really mean that all those peer-thread death
notifications should be exposed at the systemtap script level.  If the
probe point says "process().exec" or "process().death", then one could
construe that as only a single process-level notification being
requested.  (Then how to receive a notification about each individual
thread terminating?  A new probe point perhaps; or maybe it already
shows up under process("...").syscall?)

- FChE

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: systemtap bug #6500 fallout
  2008-05-13  0:00   ` Frank Ch. Eigler
@ 2008-05-13 10:43     ` Ananth N Mavinakayanahalli
  2008-05-14 12:35       ` Frank Ch. Eigler
  0 siblings, 1 reply; 5+ messages in thread
From: Ananth N Mavinakayanahalli @ 2008-05-13 10:43 UTC (permalink / raw)
  To: Frank Ch. Eigler; +Cc: David Smith, Systemtap List

On Mon, May 12, 2008 at 04:23:50PM -0400, Frank Ch. Eigler wrote:
> 
> I wrote:
> 
> >> [...]
> >> So, my tentative plan is to have the task finder layer call the callback
> >>  (potentially) twice when an exec happens - once to unregister the "old"
> >> path and once to register the "new" path.
> >
> > Yeah, that makes sense.  Further, since an "exec" is a process-wide
> > event, so I'd expect death notifications for all threads other than
> > the caller.  An "exit" is also process-wide; I presume the per-thread
> > death notifications work there already.  [...]
> 
> By the way, this does not really mean that all those peer-thread death
> notifications should be exposed at the systemtap script level.  If the
> probe point says "process().exec" or "process().death", then one could
> construe that as only a single process-level notification being
> requested.  (Then how to receive a notification about each individual
> thread terminating?  A new probe point perhaps; or maybe it already
> shows up under process("...").syscall?)

Yeah, thread creation and termination notifications are important. Maybe
there can be a process("...").thread-monitor or some such extension that
specifically follows this?

Ananth

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: systemtap bug #6500 fallout
  2008-05-13 10:43     ` Ananth N Mavinakayanahalli
@ 2008-05-14 12:35       ` Frank Ch. Eigler
  0 siblings, 0 replies; 5+ messages in thread
From: Frank Ch. Eigler @ 2008-05-14 12:35 UTC (permalink / raw)
  To: Ananth N Mavinakayanahalli; +Cc: systemtap

Hi -

On Tue, May 13, 2008 at 12:05:41PM +0530, Ananth N Mavinakayanahalli wrote:

> [...]  Yeah, thread creation and termination notifications are
> important. Maybe there can be a process("...").thread-monitor or
> some such extension that specifically follows this?

It may be most useful to have new probe points specifically for thread
lifetime notification too: 
    process("...").thread.clone process("...").thread.death
and probably just rename
    process("...").clone to process("...").fork

- FChE

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-05-13 10:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-05-12 23:32 systemtap bug #6500 fallout David Smith
2008-05-12 23:41 ` Frank Ch. Eigler
2008-05-13  0:00   ` Frank Ch. Eigler
2008-05-13 10:43     ` Ananth N Mavinakayanahalli
2008-05-14 12:35       ` Frank Ch. Eigler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).