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

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

@@ -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;
}

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   `------------------------

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

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

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

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

--On Wednesday, March 13, 2002 07:26:56 PM -0800 David Rees

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
[email protected] T/L 678-3059

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

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

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.

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

Daniel

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: [email protected]
Current:   [email protected]

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.

Daniel

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

No. gprof is uselss in today world.

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

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

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

Interesting. Care to share your reason for this assumption?

Servus,
       Daniel

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--

Hi Ulrich,

Why do you think profiling is useless in todays world?

Regards, Olaf.