Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Dan Kegel]

http://bugs.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=default&pr=1427

IMHO gprof not working on Linux is getting to be
annoying for embedded developers adopting Linux.
The workaround is to have each new thread call setitimer.  
Users can do this in their programs (see
http://sam.zoy.org/doc/programming/gprof.html )
but it's not well known for some reason.  Perhaps everyone
figures if the workaround was so easy, it would have been fixed
in glibc long ago.

So let's break the logjam and fix glibc's linuxthreads' pthread_create to do
this.
Here's an untested patch that changes pthread_create
to sense (using sigaction) whether profiling is
active, and if so, arranges for the new thread to 
transparantly call setitimer before calling the user's
thread main.

Comments welcome...
- Dan

--- glibc-2.2.4/linuxthreads/pthread.c.orig     Wed Mar 13 12:01:42 2002
+++ glibc-2.2.4/linuxthreads/pthread.c  Wed Mar 13 12:31:23 2002
@@ -637,15 +637,73 @@
 
 /* Thread creation */
 
+/* profiling support: struct used to tell child thread how to call setitimer
*/
+typedef struct pthread_create_wrapper_s {
+  void *(*start_routine) (void *);
+  void *arg;
+
+  pthread_mutex_t lock;
+  pthread_cond_t wait;
+
+  struct itimerval itimer;
+} pthread_create_wrapper_t;
+
+/* profiling support: wrapper around child thread start routine to call
setitimer */
+static void *pthread_create_start_wrapper(void *data)
+{
+  /* Put user data in thread-local variables */
+  void *(*start_routine) (void *) = ((pthread_create_wrapper_t *)
data)->start_routine;
+  void *arg = ((pthread_create_wrapper_t *) data)->arg;
+
+  /* Turn on a profiling timer for this new thread. */
+  setitimer(ITIMER_PROF, &((pthread_create_wrapper_t *) data)->itimer, NULL);
+
+  /* Tell the calling thread that we don't need its data anymore */
+  __pthread_mutex_lock(&((pthread_create_wrapper_t *) data)->lock);
+  __pthread_cond_signal(&((pthread_create_wrapper_t *) data)->wait);
+  __pthread_mutex_unlock(&((pthread_create_wrapper_t *) data)->lock);
+
+  /* Call the real function */
+  return start_routine(arg);
+}
+
 int __pthread_create_2_1(pthread_t *thread, const pthread_attr_t *attr,
                         void * (*start_routine)(void *), void *arg)
 {
+  pthread_create_wrapper_t wrapper_data;
+  struct sigaction oact;
+  int profiling;
+
   pthread_descr self = thread_self();
   struct pthread_request request;
   int retval;
   if (__builtin_expect (__pthread_manager_request, 0) < 0) {
     if (__pthread_initialize_manager() < 0) return EAGAIN;
   }
+
+  /* profiling support:
+   * See if profiling is on.
+   * We know glibc uses sigaction for profiling,
+   * so it's safe to check sa_sigaction.
+   */
+  profiling = 0;
+  if (0 == sigaction(SIGPROF, 0, &oact)
+  &&  (oact.sa_sigaction != SIG_IGN)
+  &&  (oact.sa_sigaction != SIG_DFL)) {
+    profiling = 1;
+
+    /* Arrange for child thread to call setitimer so his cpu time will
+     * be profiled
+     */
+    wrapper_data.start_routine = start_routine;
+    start_routine = pthread_create_start_wrapper;
+    wrapper_data.arg = arg;
+    getitimer(ITIMER_PROF, &wrapper_data.itimer);
+    __pthread_cond_init(&wrapper_data.wait, NULL);
+    __pthread_mutex_init(&wrapper_data.lock, NULL);
+    __pthread_mutex_lock(&wrapper_data.lock);
+  }
+
   request.req_thread = self;
   request.req_kind = REQ_CREATE;
   request.req_args.create.attr = attr;
@@ -658,6 +716,18 @@
   retval = THREAD_GETMEM(self, p_retcode);
   if (__builtin_expect (retval, 0) == 0)
     *thread = (pthread_t) THREAD_GETMEM(self, p_retval);
+
+  /* profiling support:
+   * Wait for child thread to call setitimer, then free resources
+   */
+  if (profiling) {
+    if (retval == 0)
+      __pthread_cond_wait(&wrapper_data.wait, &wrapper_data.lock);
+    __pthread_mutex_unlock(&wrapper_data.lock);
+    __pthread_mutex_destroy(&wrapper_data.lock);
+    __pthread_cond_destroy(&wrapper_data.wait);
+  }
+
   return retval;
 }

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Ulrich Drepper]

[-- Attachment #1: Type: text/plain, Size: 545 bytes --]

On Wed, 2002-03-13 at 15:17, Dan Kegel wrote:

> So let's break the logjam and fix glibc's linuxthreads' pthread_create to do
> this.

I will add nothing like this.  The implementation is broken enough and
any addition just makes it worse.  If you patch your own code you'll get
what you want at your own risk.

-- 
---------------.                          ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Red Hat          `--' drepper at redhat.com   `------------------------

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 232 bytes --]

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Dan Kegel]

Ulrich Drepper wrote:
> 
> On Wed, 2002-03-13 at 15:17, Dan Kegel wrote:
> 
> > So let's break the logjam and fix glibc's linuxthreads' pthread_create
> > to [support profiling multithreaded programs]
> 
> I will add nothing like this.  The implementation is broken enough and
> any addition just makes it worse.  If you patch your own code you'll get
> what you want at your own risk.

OK.  What's the right way to fix this, then?

Here are a few alternate ideas off the top of my head:

* Rip out Linuxthreads, replace it with NGPT, and
start fixing from there?  (Or does NGPT already fix this?)

* Rewrite Linux's setitimer(ITIMER_PROF,...) to set up an 
interval timer for all threads of the thread group.

* Implement the profil() system call from Solaris
( http://ua1vm.ua.edu/cgi-bin/man-cgi?profil+2 )

What's your favorite idea for getting profiling of
multithreaded programs working on Linux?

Thanks,
Dan

Re: libc/1427: gprof does not profile threads <synopsis of the problem

[Alan Cox]

> Here are a few alternate ideas off the top of my head:
> 
> * Rip out Linuxthreads, replace it with NGPT, and
> start fixing from there?  (Or does NGPT already fix this?)
> 
> * Rewrite Linux's setitimer(ITIMER_PROF,...) to set up an 
> interval timer for all threads of the thread group.
> 
> * Implement the profil() system call from Solaris
> ( http://ua1vm.ua.edu/cgi-bin/man-cgi?profil+2 )
> 
> What's your favorite idea for getting profiling of
> multithreaded programs working on Linux?

Kernel support is not needed  for this, do it in user space. Or prove it
has to be in kernel space

Re: libc/1427: gprof does not profile threads <synopsis of the problem

[Dan Kegel]

Alan Cox wrote:
> 
> > Here are a few alternate ideas off the top of my head:
> >
> > * Rip out Linuxthreads, replace it with NGPT, and
> > start fixing from there?  (Or does NGPT already fix this?)
> >
> > * Rewrite Linux's setitimer(ITIMER_PROF,...) to set up an
> > interval timer for all threads of the thread group.
> >
> > * Implement the profil() system call from Solaris
> > ( http://ua1vm.ua.edu/cgi-bin/man-cgi?profil+2 )
> >
> > What's your favorite idea for getting profiling of
> > multithreaded programs working on Linux?
> 
> Kernel support is not needed  for this, do it in user space. Or prove it
> has to be in kernel space

I'm all in favor of a userspace fix.  I suggested a patch
to glibc to fix this.  Ulrich rejected it; I'm trying
to coax out of him how he thinks profiling of multithreaded
programs on Linux should be fixed.

I hope we can all at least agree that
  cc foo.c -pg -pthread
  ./a.out
  gprof a.out
should work in Linux without any workarounds on the part of the programmer...

- Dan


- Dan

Re: libc/1427: gprof does not profile threads <synopsis of the problem

[Alan Cox]

> I'm all in favor of a userspace fix.  I suggested a patch
> to glibc to fix this.  Ulrich rejected it; I'm trying
> to coax out of him how he thinks profiling of multithreaded
> programs on Linux should be fixed.

Good and I'll reject any kernel patches 8)

If Ulrich won't talk then talk to the NGPT people. Maybe a little
competition will warm things up.

Re: libc/1427: gprof does not profile threads <synopsis of the problem

[Jeff Garzik]

Alan Cox wrote:

>>I'm all in favor of a userspace fix.  I suggested a patch
>>to glibc to fix this.  Ulrich rejected it; I'm trying
>>to coax out of him how he thinks profiling of multithreaded
>>programs on Linux should be fixed.
>>
>
>Good and I'll reject any kernel patches 8)
>
>If Ulrich won't talk then talk to the NGPT people. Maybe a little
>competition will warm things up.
>
Talk about a small world, I just found out today someone I know has been 
maintaining the NGPT kernel patches :)

http://gtf.org/~dank/ngpt/

    Jeff

Re: libc/1427: gprof does not profile threads <synopsis of the problem

[David Rees]

On Wed, Mar 13, 2002 at 09:55:01PM -0500, Jeff Garzik wrote:
>
> Talk about a small world, I just found out today someone I know has been 
> maintaining the NGPT kernel patches :)
> 
> http://gtf.org/~dank/ngpt/

It even looks like kernel support is included 2.4.19-pre3:

http://oss.software.ibm.com/pthreads/

But don't see anything about it in any of the recent change logs...

-Dave

Re: libc/1427: gprof does not profile threads <synopsis of the problem

[Dave McCracken]

--On Wednesday, March 13, 2002 07:26:56 PM -0800 David Rees
<dbr@greenhydrant.com> wrote:

> On Wed, Mar 13, 2002 at 09:55:01PM -0500, Jeff Garzik wrote:
>> 
>> Talk about a small world, I just found out today someone I know has been 
>> maintaining the NGPT kernel patches :)
>> 
>> http://gtf.org/~dank/ngpt/
> 
> It even looks like kernel support is included 2.4.19-pre3:
> 
> http://oss.software.ibm.com/pthreads/
> 
> But don't see anything about it in any of the recent change logs...

The relevant line from the changelog is:

- Signal changes for thread groups			(Dave McCracken)

This is the only patch that NGPT needs to work.

Dave McCracken

======================================================================
Dave McCracken          IBM Linux Base Kernel Team      1-512-838-3059
dmccr@us.ibm.com                                        T/L   678-3059

Re: libc/1427: gprof does not profile threads <synopsis of the problem

[Daniel Phillips]

On March 14, 2002 02:08 am, Dan Kegel wrote:
> Alan Cox wrote:
> > Kernel support is not needed  for this, do it in user space. Or prove it
> > has to be in kernel space
> 
> I'm all in favor of a userspace fix.  I suggested a patch
> to glibc to fix this.  Ulrich rejected it; I'm trying
> to coax out of him how he thinks profiling of multithreaded
> programs on Linux should be fixed.

I find it odd he vetoed your fix and didn't suggest an alternative.

/me checks to make sure this is cc'd to Ulrich.

-- 
Daniel

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Jakub Jelinek]

On Wed, Mar 13, 2002 at 04:19:02PM -0800, Dan Kegel wrote:
> Ulrich Drepper wrote:
> > 
> > On Wed, 2002-03-13 at 15:17, Dan Kegel wrote:
> > 
> > > So let's break the logjam and fix glibc's linuxthreads' pthread_create
> > > to [support profiling multithreaded programs]
> > 
> > I will add nothing like this.  The implementation is broken enough and
> > any addition just makes it worse.  If you patch your own code you'll get
> > what you want at your own risk.
> 
> OK.  What's the right way to fix this, then?

Surely don't use timer for profiling.
Have the compiler generate profiling calls both at function entry and exit
and use rdtsc/whatever other register your machine has (or even better
profiling registers) to note time of that function being entered/left.

	Jakub

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[John Levon]

On Thu, Mar 14, 2002 at 02:08:34AM -0500, Jakub Jelinek wrote:

> Surely don't use timer for profiling.
> Have the compiler generate profiling calls both at function entry and exit
> and use rdtsc/whatever other register your machine has (or even better
> profiling registers) to note time of that function being entered/left.

http://www710.univ-lyon1.fr/~yperret/fnccheck/

regards
john

-- 
I am a complete moron for forgetting about endianness. May I be
forever marked as such.

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Dan Kegel]

Jakub Jelinek wrote:
> 
> On Wed, Mar 13, 2002 at 04:19:02PM -0800, Dan Kegel wrote:
> > Ulrich Drepper wrote:
> > >
> > > On Wed, 2002-03-13 at 15:17, Dan Kegel wrote:
> > >
> > > > So let's break the logjam and fix glibc's linuxthreads' pthread_create
> > > > to [support profiling multithreaded programs]
> > >
> > > I will add nothing like this.  The implementation is broken enough and
> > > any addition just makes it worse.  If you patch your own code you'll get
> > > what you want at your own risk.
> >
> > OK.  What's the right way to fix this, then?
> 
> Surely don't use timer for profiling.
> Have the compiler generate profiling calls both at function entry and exit
> and use rdtsc/whatever other register your machine has (or even better
> profiling registers) to note time of that function being entered/left.

That's a different style of profiling.  There is a fellow
working on that (see http://sourceforge.net/projects/fnccheck )
but there's value in the gprof-style of profiling as well.
(For instance, I suspect it's lower overhead.)

I think it's reasonable for us to get gprof working; it's
a useful part of classic Unix, and the fix looks easy.

Ulrich, do you at least agree that it would be desirable for
gprof to work properly on multithreaded programs?

- Dan

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Ulrich Drepper]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

On Fri, 2002-03-15 at 13:56, Dan Kegel wrote:

> Ulrich, do you at least agree that it would be desirable for
> gprof to work properly on multithreaded programs?

No.  gprof is uselss in today world.

-- 
---------------.                          ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Red Hat          `--' drepper at redhat.com   `------------------------

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 232 bytes --]

Re: libc/1427: gprof does not profile threads <synopsis of the

[Alan Cox]

> > Ulrich, do you at least agree that it would be desirable for
> > gprof to work properly on multithreaded programs?
> 
> No.  gprof is uselss in today world.

Now you know why I suggested talking to the NGPT people and the gprof
maintainer

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Erik Andersen]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On Fri Mar 15, 2002 at 04:19:17PM -0800, Ulrich Drepper wrote:
> On Fri, 2002-03-15 at 13:56, Dan Kegel wrote:
> 
> > Ulrich, do you at least agree that it would be desirable for
> > gprof to work properly on multithreaded programs?
> 
> No.  gprof is uselss in today world.

Then why not change sysdeps/generic/initfini.c with something like:

-      if (gmon_start)
+      if (gmon_start && __pthread_initialize_minimal)
          gmon_start ();

So it doesn't even try when threading?

 -Erik

--
Erik B. Andersen             http://codepoet-consulting.com/
--This message was written using 73% post-consumer electrons--

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Olaf Dietsche]

Hi Ulrich,

Ulrich Drepper <drepper@redhat.com> writes:

> On Fri, 2002-03-15 at 13:56, Dan Kegel wrote:
>
>> Ulrich, do you at least agree that it would be desirable for
>> gprof to work properly on multithreaded programs?
>
> No.  gprof is uselss in today world.

Why do you think profiling is useless in todays world?

Regards, Olaf.

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Daniel Egger]

Am Sam, 2002-03-16 um 01.19 schrieb Ulrich Drepper:

> > Ulrich, do you at least agree that it would be desirable for
> > gprof to work properly on multithreaded programs?
 
> No.  gprof is uselss in today world.

Interesting. Care to share your reason for this assumption?

-- 
Servus,
       Daniel

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Daniel Phillips]

On March 14, 2002 01:19 am, Dan Kegel wrote:
> Ulrich Drepper wrote:
> > On Wed, 2002-03-13 at 15:17, Dan Kegel wrote:
> > 
> > > So let's break the logjam and fix glibc's linuxthreads' pthread_create
> > > to [support profiling multithreaded programs]
> > 
> > I will add nothing like this.  The implementation is broken enough and
> > any addition just makes it worse.  If you patch your own code you'll get
> > what you want at your own risk.
> 
> OK.  What's the right way to fix this, then?

I see, he said to patch your own code and probably feels the issue is done with.
Color me less than impressed.

-- 
Daniel

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Richard Gooch]

Daniel Phillips writes:
> On March 14, 2002 01:19 am, Dan Kegel wrote:
> > Ulrich Drepper wrote:
> > > On Wed, 2002-03-13 at 15:17, Dan Kegel wrote:
> > > 
> > > > So let's break the logjam and fix glibc's linuxthreads' pthread_create
> > > > to [support profiling multithreaded programs]
> > > 
> > > I will add nothing like this.  The implementation is broken enough and
> > > any addition just makes it worse.  If you patch your own code you'll get
> > > what you want at your own risk.
> > 
> > OK.  What's the right way to fix this, then?
> 
> I see, he said to patch your own code and probably feels the issue
> is done with.  Color me less than impressed.

Ulrich tends to take a hardline, "must be 100% correct" approach to
things. He doesn't seem to like 99% solutions that will work most of
the time but not always. This does cause some friction with people who
want something that works "most of the time" (aka "good enough"). But
before we cast stones, let's not forget that in kernel-land we see
similar attitudes. How many patches has Linus rejected because it's
"not the right way", even if many users really want it?

I guess there's always a difference between coding up and submitting
an "unclean" workaround/fixup for someone else's code, or having it
applied to your own :-)

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: libc/1427: gprof does not profile threads <synopsis of the problem (one li\ne)>

[Daniel Phillips]

On March 14, 2002 05:25 pm, Richard Gooch wrote:
> Daniel Phillips writes:
> > On March 14, 2002 01:19 am, Dan Kegel wrote:
> > > Ulrich Drepper wrote:
> > > > On Wed, 2002-03-13 at 15:17, Dan Kegel wrote:
> > > > 
> > > > > So let's break the logjam and fix glibc's linuxthreads'
> > > > > pthread_create to [support profiling multithreaded programs]
> > > > 
> > > > I will add nothing like this.  The implementation is broken enough and
> > > > any addition just makes it worse.  If you patch your own code you'll 
> > > > get what you want at your own risk.
> > > 
> > > OK.  What's the right way to fix this, then?
> > 
> > I see, he said to patch your own code and probably feels the issue
> > is done with.  Color me less than impressed.
> 
> Ulrich tends to take a hardline, "must be 100% correct" approach to
> things. He doesn't seem to like 99% solutions that will work most of
> the time but not always. This does cause some friction with people who
> want something that works "most of the time" (aka "good enough"). But
> before we cast stones, let's not forget that in kernel-land we see
> similar attitudes. How many patches has Linus rejected because it's
> "not the right way", even if many users really want it?

Oh, I have no trouble with the 'must be 100%' rule, but the failing to define 
what '100%' actually means is... um... not the way Linus would handle it.

Failing to engage in discourse is just not the 'open' way.

> I guess there's always a difference between coding up and submitting
> an "unclean" workaround/fixup for someone else's code, or having it
> applied to your own :-)

-- 
Daniel