|
From: Bart V. A. <bar...@gm...> - 2006-12-12 13:11:52
|
By adding tracing in the drd tool I found out that Valgrind's core first calls VG_TRACK(thread_run)() before it calls VG_TRACK(post_thread_create)() when a new thread is created. This surprised me. Is this intended behavior ? The first tracking function is called from coregrind/m_scheduler/scheduler.c, function VG_(set_running)(). The second tracking function is called from coregrind/m_syswrap/syswrap-linux.c, function thread_wrapper(). thread_wrapper() calls VG_(set_running)() before it calls VG_TRACK(post_thread_create)(). Would it be wise to switch the calling order of VG_(set_running)() and VG_TRACK(post_thread_create)() ? |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Julian S. <js...@ac...> - 2006-12-17 17:39:11
|
I had a look at this just now. On Tuesday 12 December 2006 13:11, Bart Van Assche wrote: > By adding tracing in the drd tool I found out that Valgrind's core > first calls VG_TRACK(thread_run)() before it calls > VG_TRACK(post_thread_create)() when a new thread is created. This > surprised me. Is this intended behavior ? The first tracking function > is called from coregrind/m_scheduler/scheduler.c, function > VG_(set_running)(). The second tracking function is called from > coregrind/m_syswrap/syswrap-linux.c, function thread_wrapper(). > thread_wrapper() calls VG_(set_running)() before it calls > VG_TRACK(post_thread_create)(). Would it be wise to switch the calling > order of VG_(set_running)() and VG_TRACK(post_thread_create)() ? Well, no .. I don't think so. The problem is that V uses a big lock to ensure everything is serialised. thread_wrapper() is one of the very few places where there is genuinely > 1 kernel thread runnable, because that's what V asks sys_clone() to run when the client does sys_clone(). So basically the first thing that thread_wrapper() does is to acquire the big lock. Before that is done, it isn't safe to touch any global state of the system. Note that VG_(set_running) and VG_(set_sleeping) are poorly named. I should rename to them to VG_(acquire_biglock) and VG_(release_biglock) respectively. Anyway: does the existing sequence cause a problem for drd? J |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Julian S. <js...@ac...> - 2006-12-17 19:02:23
|
> Note that VG_(set_running) and VG_(set_sleeping) are poorly named.
> I should rename to them to VG_(acquire_biglock) and VG_(release_biglock)
> respectively.
Ok, did that (r6408). This makes it clearer that
(1) Valgrind has a Big Lock ("the_BigLock" in scheduler.c)
(2) Before doing basically anything at all, whether running
the core, the tool, or JIT-generated code, a thread must
hold the lock: it must have gone past VG_(acquire_BigLock)
but not have gone past VG_(release_BigLock).
J
|
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Bart V. A. <bar...@gm...> - 2006-12-17 20:28:55
|
On 12/17/06, Julian Seward <js...@ac...> wrote: > > > Would it be wise to switch the calling > > order of VG_(set_running)() and VG_TRACK(post_thread_create)() ? > > Well, no .. I don't think so. The problem is that V uses a big > lock to ensure everything is serialised. thread_wrapper() is > one of the very few places where there is genuinely > 1 kernel > thread runnable, because that's what V asks sys_clone() to run when > the client does sys_clone(). So basically the first thing that > thread_wrapper() does is to acquire the big lock. Before that is > done, it isn't safe to touch any global state of the system. > > Note that VG_(set_running) and VG_(set_sleeping) are poorly named. > I should rename to them to VG_(acquire_biglock) and VG_(release_biglock) > respectively. > > Anyway: does the existing sequence cause a problem for drd? The current behavior is not easy to work with in drd: the drd tool can only create a thread record if it has both the ThreadId of the newly created thread and the ThreadId of the parent thread. This information is only passed to tools via VG_TRACK(post_thread_create)(), and not via VG_TRACK(thread_run)(). Or: I would have to ignore any VG_TRACK(thread_run)() calls that happen before VG_TRACK(post_thread_create)(). This is something that should be done by the core IMHO. Bart. |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Julian S. <js...@ac...> - 2006-12-18 01:24:48
|
> > Anyway: does the existing sequence cause a problem for drd?
>
> The current behavior is not easy to work with in drd: the drd tool can
> only create a thread record if it has both the ThreadId of the newly
> created thread and the ThreadId of the parent thread. This information
> is only passed to tools via VG_TRACK(post_thread_create)(), and not
> via VG_TRACK(thread_run)(). Or: I would have to ignore any
> VG_TRACK(thread_run)() calls that happen before
> VG_TRACK(post_thread_create)(). This is something that should be done
> by the core IMHO.
I agree, the core is wrong. The problem is that the call
VG_TRACK( thread_run, tid ) in scheduler.c is done too early;
it is done as soon as the thread has the CPU lock, and a lot
can happen between getting the lock and running any code of
the thread. Maybe better would be to do it just before the
thread's code is really run.
Can you try the following patch? I think it should cause
post_thread_create to happen before thread_run, which is what
you want AIUI.
Do you also need a thread-has-stopped-running event? This
might be useful since presently a tool can only know which was
the last thread to run. It cannot know that no thread is running,
because it is only told that a thread is running, and is never told
when that thread stops running. Although it would require a change
to the core/tool interface.
Josef, I see that callgrind also uses the thread_run event.
Any problem if that event is moved to be a lot closer in time
to when a thread really runs?
J
Index: coregrind/m_scheduler/scheduler.c
===================================================================
--- coregrind/m_scheduler/scheduler.c (revision 6408)
+++ coregrind/m_scheduler/scheduler.c (working copy)
@@ -233,10 +233,6 @@
VG_(sprintf)(buf, " acquired lock (%s)", who);
print_sched_event(tid, buf);
}
-
- // While thre modeling is disable, issue thread_run events here
- // VG_(tm_thread_switchto)(tid);
- VG_TRACK( thread_run, tid );
}
/*
@@ -616,6 +612,9 @@
VG_(printf)("\n");
}
+ // Tell the tool this thread is about to run
+ VG_TRACK( thread_run, tid );
+
vg_assert(VG_(in_generated_code) == False);
VG_(in_generated_code) = True;
@@ -636,6 +635,8 @@
block_signals(tid);
}
+ // XXX if needed, we could tell the tool this thread has finished running
+
done_this_time = (Int)dispatch_ctr_SAVED - (Int)VG_(dispatch_ctr) - 0;
vg_assert(done_this_time >= 0);
@@ -652,6 +653,7 @@
volatile Int jumped;
volatile ThreadState* tst;
volatile UWord argblock[4];
+ volatile UInt retval;
/* Paranoia */
vg_assert(VG_(is_valid_tid)(tid));
@@ -686,6 +688,9 @@
argblock[2] = 0; /* next guest IP is written here */
argblock[3] = 0; /* guest state ptr afterwards is written here */
+ // Tell the tool this thread is about to run
+ VG_TRACK( thread_run, tid );
+
vg_assert(VG_(in_generated_code) == False);
VG_(in_generated_code) = True;
@@ -703,16 +708,20 @@
vg_assert(argblock[2] == 0); /* next guest IP was not written */
vg_assert(argblock[3] == 0); /* trc was not written */
block_signals(tid);
- return VG_TRC_FAULT_SIGNAL;
+ retval = VG_TRC_FAULT_SIGNAL;
} else {
/* store away the guest program counter */
VG_(set_IP)( tid, argblock[2] );
if (argblock[3] == argblock[1])
/* the guest state pointer afterwards was unchanged */
- return VG_TRC_BORING;
+ retval = VG_TRC_BORING;
else
- return (UInt)argblock[3];
+ retval = (UInt)argblock[3];
}
+
+ // XXX if needed, we could tell the tool this thread has finished running
+
+ return retval;
}
|
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Nicholas N. <nj...@cs...> - 2006-12-18 09:19:23
|
On Mon, 18 Dec 2006, Julian Seward wrote: > Do you also need a thread-has-stopped-running event? This > might be useful since presently a tool can only know which was > the last thread to run. It cannot know that no thread is running, > because it is only told that a thread is running, and is never told > when that thread stops running. Although it would require a change > to the core/tool interface. The events part of the interface needs to change a bit for Bart's changes anyway, so this is not a problem. Seems like a good idea. Nick |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Bart V. A. <bar...@gm...> - 2006-12-18 11:54:21
|
On 12/18/06, Julian Seward <js...@ac...> wrote: > > Do you also need a thread-has-stopped-running event? This > might be useful since presently a tool can only know which was > the last thread to run. It cannot know that no thread is running, > because it is only told that a thread is running, and is never told > when that thread stops running. Although it would require a change > to the core/tool interface. > > J I already defined such an event, and called it thread_finished. See also the most recent drd patch I posted (http://home.euphonynet.be/bvassche/valgrind/valgrind-6397-drd-2006-12-13.patch.gz). Bart. |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Bart V. A. <bar...@gm...> - 2006-12-18 17:48:47
|
On 12/18/06, Julian Seward <js...@ac...> wrote: > > Can you try the following patch? I think it should cause > post_thread_create to happen before thread_run, which is what > you want AIUI. > > Do you also need a thread-has-stopped-running event? This > might be useful since presently a tool can only know which was > the last thread to run. It cannot know that no thread is running, > because it is only told that a thread is running, and is never told > when that thread stops running. Although it would require a change > to the core/tool interface. [ ... ] > > Index: coregrind/m_scheduler/scheduler.c > =================================================================== > --- coregrind/m_scheduler/scheduler.c (revision 6408) > +++ coregrind/m_scheduler/scheduler.c (working copy) The drd tool works fine with this patch (I removed my own patch that caused thread_post_create to be called earlier). If VG_TRACK(thread_finished)() could be called from scheduler.c instead of m_syswrap, that would be a nice improvement as well. Bart. |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Nicholas N. <nj...@cs...> - 2006-12-18 05:16:59
|
On Sun, 17 Dec 2006, Bart Van Assche wrote: > The current behavior is not easy to work with in drd: the drd tool can > only create a thread record if it has both the ThreadId of the newly > created thread and the ThreadId of the parent thread. This information > is only passed to tools via VG_TRACK(post_thread_create)(), and not > via VG_TRACK(thread_run)(). Or: I would have to ignore any > VG_TRACK(thread_run)() calls that happen before > VG_TRACK(post_thread_create)(). This is something that should be done > by the core IMHO. Don't you have a core change in your patch that addresses this? Nick |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Josef W. <Jos...@gm...> - 2006-12-18 09:30:09
|
On Monday 18 December 2006 02:34, Julian Seward wrote: > Do you also need a thread-has-stopped-running event? This > might be useful since presently a tool can only know which was > the last thread to run. It cannot know that no thread is running, > because it is only told that a thread is running, and is never told > when that thread stops running. Although it would require a change > to the core/tool interface. > > Josef, I see that callgrind also uses the thread_run event. > Any problem if that event is moved to be a lot closer in time > to when a thread really runs? I use this event to track thread switches, and, totally unrelated, to do polling for a command (for interactivity with callgrind_control). The latter really should be done elsewhere. For the first use, I actually also check the thread ID at the beginning of every basic block, so tracking thread_run actually is not needed for this. This is wasteful, but unfortunately necessary; VG_(thread_run) is not exhaustive for detecting thread switches. There was some case with a signal handler running in a new thread, and longjmp'ing into normal code at the end. The latter, Valgrind can not detect itself. However, I have to look this up again. I can not remember the exact problem; it was with code generated by an ADA compiler. But regarding the order of VG_TRACK(thread_run)() and VG_TRACK(post_thread_create)(), I see no problem with callgrind. Cheers, Josef |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Bart V. A. <bar...@gm...> - 2006-12-18 12:01:33
|
On 12/18/06, Nicholas Nethercote <nj...@cs...> wrote: > On Sun, 17 Dec 2006, Bart Van Assche wrote: > > > The current behavior is not easy to work with in drd: the drd tool can > > only create a thread record if it has both the ThreadId of the newly > > created thread and the ThreadId of the parent thread. This information > > is only passed to tools via VG_TRACK(post_thread_create)(), and not > > via VG_TRACK(thread_run)(). Or: I would have to ignore any > > VG_TRACK(thread_run)() calls that happen before > > VG_TRACK(post_thread_create)(). This is something that should be done > > by the core IMHO. > > Don't you have a core change in your patch that addresses this? > > Nick What I did in the drd patch is to move the call to VG_TRACK ( post_thread_create, tst->os_state.parent, tid ); just in front of the call to VG_(set_running)(...). But if I understood Julian correctly, the purpose of this last call is to acquire the "big lock". I'm afraid that this means that I introduced a race condition this way, and that I should revert this change. Bart. |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Julian S. <js...@ac...> - 2006-12-18 14:22:48
|
> > Don't you have a core change in your patch that addresses this? > > > > Nick > > What I did in the drd patch is to move the call to VG_TRACK ( > post_thread_create, tst->os_state.parent, tid ); just in front of the > call to VG_(set_running)(...). But if I understood Julian correctly, > the purpose of this last call is to acquire the "big lock". I'm afraid > that this means that I introduced a race condition this way, and that > I should revert this change. Yes ... can you try my proposed patch instead? If it is OK I'll commit it. J |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Julian S. <js...@ac...> - 2006-12-18 14:34:55
|
On Monday 18 December 2006 09:29, Josef Weidendorfer wrote: > This is wasteful, but unfortunately necessary; > VG_(thread_run) is not exhaustive for detecting thread > switches. There was some case with a signal handler > running in a new thread, and longjmp'ing into normal > code at the end. The latter, Valgrind can not detect itself. > However, I have to look this up again. I can not remember > the exact problem; it was with code generated by an ADA > compiler. That sounds like a bug in the core. You should have complained earlier :-) My proposed patch should make it possible for callgrind/drd to know exactly when client code is running and when it is not, and which thread it is. And no problems with longjmp etc. One thing to note (Bart too) is that the proposed change will cause VG_TRACK(thread_run) and the proposed VG_TRACK(thread_stopped) to be called very often, perhaps 1000 x more often than thread_run is called at present, in the worst case. So the callbacks in the callgrind/drd need to be fast. One other thing to note: there is a subtle change in semantics too. So far, a call to VG_TRACK(thread_run)(tid) means "thread tid has acquired the big CPU lock (and so can make translations, etc, all possible actions, as well as running client code)". In the proposed new scheme, the meaning is a subset: VG_TRACK(thread_run)(tid) means "thread tid has the CPU lock and is about to start running client code". VG_TRACK(thread_stopped)(tid) means "thread tid, which previously had the CPU lock and was running client code, is no longer running client code." J |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Josef W. <Jos...@gm...> - 2006-12-18 23:07:56
|
On Monday 18 December 2006 15:44, Julian Seward wrote: > On Monday 18 December 2006 09:29, Josef Weidendorfer wrote: > > This is wasteful, but unfortunately necessary; > > VG_(thread_run) is not exhaustive for detecting thread > > switches. There was some case with a signal handler > > running in a new thread, and longjmp'ing into normal > > code at the end. The latter, Valgrind can not detect itself. > > However, I have to look this up again. I can not remember > > the exact problem; it was with code generated by an ADA > > compiler. > > That sounds like a bug in the core. You should have complained > earlier :-) Ok, I tried to reconstruct it. It was at VG 2.x times. There actually were 2 problems: (1) SK_(post_signal) was not being called exhaustively for the case that a signal handler was left via longjmp. And callgrind needs a signal handler exit event. I can not see how (also 3.x) VG ever could catch this event, as the signal frame is discarded by the longjmp. However, this has nothing to do with (2) There was a problem in older VG 2.x releases about not notifying the tool about current thread after running a signal handler in another thread. I think this was fixed, and probably is not a problem at all in VG 3.x. I remember that (1) was nastly, as I got the ADA binary from the bug reporter, and never was able to construct a test case in source, but only could observe the problem from running the binary in VG. So there probably is no problem. However, I can check with an assertion if expected tid is the same as actual running tid. > My proposed patch should make it possible for callgrind/drd to > know exactly when client code is running and when it is not, and > which thread it is. And no problems with longjmp etc. > > One thing to note (Bart too) is that the proposed change will > cause VG_TRACK(thread_run) and the proposed VG_TRACK(thread_stopped) > to be called very often, perhaps 1000 x more often than thread_run is > called at present, in the worst case. So the callbacks in the > callgrind/drd need to be fast. Thanks. I probably have to adjust the polling interval for the command file. It would be nice if we could come up with a general way to interact with VG tools from the outside. > > One other thing to note: there is a subtle change in semantics > too. So far, a call to VG_TRACK(thread_run)(tid) means "thread tid > has acquired the big CPU lock (and so can make translations, etc, > all possible actions, as well as running client code)". In the proposed > new scheme, the meaning is a subset: > > VG_TRACK(thread_run)(tid) means "thread tid has the CPU lock and is > about to start running client code". > > VG_TRACK(thread_stopped)(tid) means "thread tid, which previously had the > CPU lock and was running client code, is no longer running client code." So the difference is that now if translation is happening, I get a VG_TRACK(thread_stopped) before, and after the translation has finished, I get a VG_TRACK(thread_run), even when not switching client threads? Josef |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Julian S. <js...@ac...> - 2006-12-19 00:08:51
|
Hi Josef. Thanks for the analysis. > Ok, I tried to reconstruct it. > It was at VG 2.x times. There actually were 2 problems: > (1) SK_(post_signal) was not being called exhaustively for the > case that a signal handler was left via longjmp. And callgrind > needs a signal handler exit event. > I can not see how (also 3.x) VG ever could catch this event, as > the signal frame is discarded by the longjmp. I agree. > However, this > has nothing to do with > (2) There was a problem in older VG 2.x releases about not > notifying the tool about current thread after running a > signal handler in another thread. I think this was fixed, > and probably is not a problem at all in VG 3.x. Good. I agree, this is probably fixed. > > VG_TRACK(thread_run)(tid) means "thread tid has the CPU lock and is > > about to start running client code". > > > > VG_TRACK(thread_stopped)(tid) means "thread tid, which previously had the > > CPU lock and was running client code, is no longer running client code." > > So the difference is that now if translation is happening, I get a > VG_TRACK(thread_stopped) before, and after the translation has finished, > I get a VG_TRACK(thread_run), even when not switching client threads? That is correct (at least, that is what I am proposing based on what Bart needs.) Is that ok for you or do you need some different behaviour? Obviously this change will go only in the trunk and not on the 3.2 branch. J |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Josef W. <Jos...@gm...> - 2006-12-19 01:19:04
|
On Tuesday 19 December 2006 01:18, Julian Seward wrote: > > > VG_TRACK(thread_run)(tid) means "thread tid has the CPU lock and is > > > about to start running client code". > > > > > > VG_TRACK(thread_stopped)(tid) means "thread tid, which previously had the > > > CPU lock and was running client code, is no longer running client code." > > > > So the difference is that now if translation is happening, I get a > > VG_TRACK(thread_stopped) before, and after the translation has finished, > > I get a VG_TRACK(thread_run), even when not switching client threads? > > That is correct (at least, that is what I am proposing based on what > Bart needs.) Ok. > Is that ok for you or do you need some different behaviour? I understand that in the beginning with lots of translations, VG_TRACK(thread_run) will be called _very_ often now. But it looks that later, in the steady case, the frequency will go down. Is it right that at the moment, VG_TRACK(thread_run) will simply be called after a fixed number of translated blocks are executed? I wonder if I can use a constant factor to keep my polling for a command file happening at regular intervals, yet not producing too much overhead. I suppose I misused this VG_TRACK(thread_run) event ... Josef |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Julian S. <js...@ac...> - 2006-12-19 02:29:50
|
> I understand that in the beginning with lots of translations, > VG_TRACK(thread_run) will be called _very_ often now. Yes. > But it looks that > later, in the steady case, the frequency will go down. Yes. > Is it right that at the moment, VG_TRACK(thread_run) will simply be > called after a fixed number of translated blocks are executed? Well, mostly, but you can't rely on it. At the moment, VG_TRACK(thread_run) is called when a thread gets the CPU lock. Ownership of the lock can change not only because of exceeding the 100k-block quota, but also if a thread does syscalls or takes a signal. > I wonder if I can use a constant factor to keep my polling for a command > file happening at regular intervals, yet not producing too much overhead. > I suppose I misused this VG_TRACK(thread_run) event ... One simple solution is for VG_TRACK(thread_run) to take a second argument, which is a 64-bit number, the total number of blocks executed by V so far. By comparing this number against the number in the previous call you can find out how much progress the system made in between. Then you could base your polling decisions on that. How does that sound? J |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Josef W. <Jos...@gm...> - 2006-12-19 10:25:06
|
On Tuesday 19 December 2006 03:39, Julian Seward wrote: > > I wonder if I can use a constant factor to keep my polling for a command > > file happening at regular intervals, yet not producing too much overhead. > > I suppose I misused this VG_TRACK(thread_run) event ... > > One simple solution is for VG_TRACK(thread_run) to take a second argument, > which is a 64-bit number, the total number of blocks executed by V so far. > By comparing this number against the number in the previous call you can > find out how much progress the system made in between. Then you could > base your polling decisions on that. How does that sound? Oh, that sounds very good. But do we need to change the signature of VG_TRACK(thread_run) for this? I still think that I am only misusing this event for my polling. Especially, the event does not trigger when the valgrind process sleeps. So a solution which waits for commands e.g. on a socket would be far better. It would be nice to have a function VG_(blocks_done)() or similar to get the total number of blocks executed so far. I have something like this in callgrind myself; but of course it is not incremented when callgrind is in "no instrumentation" mode; still, the command polling has to work to allow to switch on instrumentation. Josef > > J > |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Julian S. <js...@ac...> - 2006-12-23 01:25:50
|
Bart, Josef, I have committed in r6413, a change which I think should give Bart the sequencing you need and Josef the ability to count blocks that you need. The commit message for r6413 explains the details. Pls yell if this isn't what you wanted. Josef, there may be some cleaning up possible w.r.t clg_thread_runstate_callback that I created. I did not want to modify CLG_(run_thread) because it is called from various places, not just as a callback, so I added this 'impedance matching' function instead. J On Tuesday 19 December 2006 10:24, Josef Weidendorfer wrote: > On Tuesday 19 December 2006 03:39, Julian Seward wrote: > > > I wonder if I can use a constant factor to keep my polling for a > > > command file happening at regular intervals, yet not producing too much > > > overhead. I suppose I misused this VG_TRACK(thread_run) event ... > > > > One simple solution is for VG_TRACK(thread_run) to take a second > > argument, which is a 64-bit number, the total number of blocks executed > > by V so far. By comparing this number against the number in the previous > > call you can find out how much progress the system made in between. Then > > you could base your polling decisions on that. How does that sound? > > Oh, that sounds very good. > > But do we need to change the signature of VG_TRACK(thread_run) for this? > I still think that I am only misusing this event for my polling. > Especially, the event does not trigger when the valgrind process sleeps. So > a solution which waits for commands e.g. on a socket would be far better. > > It would be nice to have a function VG_(blocks_done)() or similar to get > the total number of blocks executed so far. > > I have something like this in callgrind myself; but of course it is not > incremented when callgrind is in "no instrumentation" mode; still, the > command polling has to work to allow to switch on instrumentation. > > Josef > > > J > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share > your opinions on IT & business topics through brief surveys - and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Bart V. A. <bar...@gm...> - 2006-12-23 12:50:20
|
On 12/23/06, Julian Seward <js...@ac...> wrote: > > Bart, Josef, I have committed in r6413, a change which I think > should give Bart the sequencing you need and Josef the ability to > count blocks that you need. The commit message for r6413 explains > the details. Pls yell if this isn't what you wanted. This patch works fine for me. Bart. |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Josef W. <Jos...@gm...> - 2006-12-23 23:06:34
|
On Saturday 23 December 2006 02:35, Julian Seward wrote:
> Josef, there may be some cleaning up possible w.r.t
> clg_thread_runstate_callback
> that I created. I did not want to modify CLG_(run_thread) because
> it is called from various places, not just as a callback, so I added
> this 'impedance matching' function instead.
Yes, thanks.
Regarding the frequency, I just checked how often the polling of
callgrind's command file was done before and after this change.
This is for startup and directly quitting xclock.
Before r6413:
:~> time strace -e open valgrind --tool=callgrind xclock &> z
real 0m8.028s
user 0m4.132s
sys 0m0.156s
:~> grep callgrind.cmd z | wc
709 7090 69130
After r6413:
:~> time strace -e open valgrind --tool=callgrind xclock &> z2
real 0m10.266s
user 0m4.840s
sys 0m1.784s
:~> grep callgrind.cmd z2 | wc
39003 390030 3822297
There were around 10 million BBs executed.
So yes, this needs some fine tuning. Polling around 200 times per user
second was already a lot, but 10000 per second is _way_ too much.
After calling CLG_(run_thread) only every 5000 BBs executed, I get
back to the old behavior:
:~> time strace -e open valgrind -v --tool=callgrind xclock &> z3
real 0m6.452s
user 0m4.132s
sys 0m0.180s
weidendo@linux:~/tmp/clg> grep callgrind.cmd z3 | wc
805 8050 78893
A lower polling frequency has to problem that the "interactivity"
gets lost if a client program is sleeping most of the time; and
that is the reason I should get rid of this polling method :-(
Josef
|
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Julian S. <js...@ac...> - 2006-12-24 14:26:32
|
While we're on the topic of thread-related stuff in the core-tool interface: I saw these in include/pub_tool_tooliface.h: void VG_(track_pre_mutex_lock)(void(*f)(ThreadId tid, Addr mutex)); void VG_(track_post_mutex_lock)(void(*f)(ThreadId tid, Addr mutex)); void VG_(track_post_mutex_unlock)(void(*f)(ThreadId tid, Addr mutex)); I'm pretty sure these are not used. From grepping I see there are no places where the core calls these (iow, no VG_TRACK(..)) calls for them. And there is no way there could be, because the core isn't intercepting pth_mutex_lock/unlock any more so it doesn't know when these events happen. So can I delete them? Bart, I presume you don't need them since drd intercepts pth_mutex_lock/unlock itself and doesn't rely on the core for that. J On Saturday 23 December 2006 23:06, Josef Weidendorfer wrote: > On Saturday 23 December 2006 02:35, Julian Seward wrote: > > Josef, there may be some cleaning up possible w.r.t > > clg_thread_runstate_callback > > that I created. I did not want to modify CLG_(run_thread) because > > it is called from various places, not just as a callback, so I added > > this 'impedance matching' function instead. > > Yes, thanks. > > Regarding the frequency, I just checked how often the polling of > callgrind's command file was done before and after this change. > > This is for startup and directly quitting xclock. > > Before r6413: > :~> time strace -e open valgrind --tool=callgrind xclock &> z > > real 0m8.028s > user 0m4.132s > sys 0m0.156s > > :~> grep callgrind.cmd z | wc > > 709 7090 69130 > > After r6413: > :~> time strace -e open valgrind --tool=callgrind xclock &> z2 > > real 0m10.266s > user 0m4.840s > sys 0m1.784s > > :~> grep callgrind.cmd z2 | wc > > 39003 390030 3822297 > > There were around 10 million BBs executed. > So yes, this needs some fine tuning. Polling around 200 times per user > second was already a lot, but 10000 per second is _way_ too much. > > After calling CLG_(run_thread) only every 5000 BBs executed, I get > > back to the old behavior: > :~> time strace -e open valgrind -v --tool=callgrind xclock &> z3 > > real 0m6.452s > user 0m4.132s > sys 0m0.180s > weidendo@linux:~/tmp/clg> grep callgrind.cmd z3 | wc > 805 8050 78893 > > A lower polling frequency has to problem that the "interactivity" > gets lost if a client program is sleeping most of the time; and > that is the reason I should get rid of this polling method :-( > > Josef > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share > your opinions on IT & business topics through brief surveys - and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Bart V. A. <bar...@gm...> - 2006-12-24 16:34:03
|
On 12/24/06, Julian Seward <js...@ac...> wrote: > > > While we're on the topic of thread-related stuff in the core-tool > interface: I saw these in include/pub_tool_tooliface.h: > > void VG_(track_pre_mutex_lock)(void(*f)(ThreadId tid, Addr mutex)); > void VG_(track_post_mutex_lock)(void(*f)(ThreadId tid, Addr mutex)); > void VG_(track_post_mutex_unlock)(void(*f)(ThreadId tid, Addr mutex)); > > I'm pretty sure these are not used. From grepping I see there are > no places where the core calls these (iow, no VG_TRACK(..)) calls for > them. And there is no way there could be, because the core isn't > intercepting pth_mutex_lock/unlock any more so it doesn't know when > these events happen. > > So can I delete them? Bart, I presume you don't need them since drd > intercepts pth_mutex_lock/unlock itself and doesn't rely on the core > for that. > > J > I think at this time only helgrind uses these tracking functions. If you have a look at the drd patch, you will see that I have removed the mutex tracking functions in the drd patch. Bart. |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Nicholas N. <nj...@cs...> - 2006-12-24 21:02:13
|
On Sun, 24 Dec 2006, Julian Seward wrote: > While we're on the topic of thread-related stuff in the core-tool > interface: I saw these in include/pub_tool_tooliface.h: > > void VG_(track_pre_mutex_lock)(void(*f)(ThreadId tid, Addr mutex)); > void VG_(track_post_mutex_lock)(void(*f)(ThreadId tid, Addr mutex)); > void VG_(track_post_mutex_unlock)(void(*f)(ThreadId tid, Addr mutex)); > > I'm pretty sure these are not used. From grepping I see there are > no places where the core calls these (iow, no VG_TRACK(..)) calls for > them. And there is no way there could be, because the core isn't > intercepting pth_mutex_lock/unlock any more so it doesn't know when > these events happen. > > So can I delete them? Bart, I presume you don't need them since drd > intercepts pth_mutex_lock/unlock itself and doesn't rely on the core > for that. Fine by me. Nick |
|
Re: [Valgrind-developers] Calling order of VG_TRACK(thread_run)()
and VG_TRACK(post_thread_create)()
From: Josef W. <Jos...@gm...> - 2006-12-25 14:14:53
|
On Sunday 24 December 2006 15:36, Julian Seward wrote: > So can I delete them? No concern from my side. Josef |