|
[Valgrind-developers] Calling order of VG_TRACK(post_mem_write)()
and VG_TRACK(post_thread_create)()
From: Bart V. A. <bar...@gm...> - 2006-12-28 10:43:28
|
For the drd tool it is important that a thread ID only is passed to the tool after it has been reported via post_thread_create that the thread exists. Apparently when a new thread is created, VG_TRACK(post_mem_write)() is called with the new thread ID before VG_TRACK(post_thread_create)() ? ==13035== by 0x38002300: drd_post_mem_write (drd_main.c:192) ==13035== by 0x380603E5: sys_set_thread_area (syswrap-x86-linux.c:656) ==13035== by 0x380610C1: vgSysWrap_x86_linux_sys_clone_before ( syswrap-x86-linux.c:290) ==13035== by 0x38026CCF: vgPlain_client_syscall (syswrap-main.c:841) ==13035== by 0x38025179: vgPlain_scheduler (scheduler.c:775) ==13035== by 0x380385CF: run_a_thread_NORETURN (syswrap-linux.c:88) Bart. |
|
From: Julian S. <js...@ac...> - 2006-12-28 17:34:42
|
Well, you certainly ask some difficult questions :-) I'm not
complaining though. This and the previous problem with
VG_TRACK(thread_run) has happened because the thread-events aspects
of the core-tool interface has not been looked at in such detail
before.
It seems to me that thread creation in V is not an atomic event,
and so we really need to split VG_TRACK(post_thread_create) into
two events: one to indicate V's intention to attempt creation of
a new thread, and one to indicate success. Possibly with names
threadid_active ( ThreadId )
post_thread_create ( ThreadId, ThreadId )
The new event, threadid_active is sent by the core right at the
start of a thread creation attempt. It tells the tool that the
specified ThreadId is now in use and that it can now expect to hear
of other events using that ThreadId.
post_thread_create remains unchanged: as at present, it notifies the
tool that a thread has been successfully created.
To make this completely general, perhaps we should also add
threadid_inactive ( ThreadId )
which tells the tool that the given ThreadId is now inactive and
any further mention of it (except for threadid_active) is an error
in the core-tool protocol.
-------
It further seems to me that this is complex enough that we should
use a state machine to specify the allowable sequences of thread
notification events in the core-tool interface. Roughly this would
be (note: this confuses 'events' with 'states')
threadid_active -> post_thread_create // if successful
threadid_active -> threadid_inactive // if failed
post_thread_create -> start_client_code // now running
start_client_code -> stop_client_code // stopping
stop_client_code -> start_client_code // running again
stop_client_code -> thread_join
thread_join -> threadid_inactive
How does all that sound to you?
J
On Thursday 28 December 2006 10:43, Bart Van Assche wrote:
> For the drd tool it is important that a thread ID only is passed to the
> tool after it has been reported via post_thread_create that the thread
> exists. Apparently when a new thread is created, VG_TRACK(post_mem_write)()
> is called with the new thread ID before VG_TRACK(post_thread_create)() ?
>
> ==13035== by 0x38002300: drd_post_mem_write (drd_main.c:192)
> ==13035== by 0x380603E5: sys_set_thread_area (syswrap-x86-linux.c:656)
> ==13035== by 0x380610C1: vgSysWrap_x86_linux_sys_clone_before (
> syswrap-x86-linux.c:290)
> ==13035== by 0x38026CCF: vgPlain_client_syscall (syswrap-main.c:841)
> ==13035== by 0x38025179: vgPlain_scheduler (scheduler.c:775)
> ==13035== by 0x380385CF: run_a_thread_NORETURN (syswrap-linux.c:88)
>
> Bart.
|
|
From: Bart V. A. <bar...@gm...> - 2006-12-29 17:46:07
|
On 12/28/06, Julian Seward <js...@ac...> wrote: > > > Well, you certainly ask some difficult questions :-) I'm not > complaining though. This and the previous problem with > VG_TRACK(thread_run) has happened because the thread-events aspects > of the core-tool interface has not been looked at in such detail > before. > > It seems to me that thread creation in V is not an atomic event, > and so we really need to split VG_TRACK(post_thread_create) into > two events: one to indicate V's intention to attempt creation of > a new thread, and one to indicate success. Possibly with names > > threadid_active ( ThreadId ) > post_thread_create ( ThreadId, ThreadId ) > > The new event, threadid_active is sent by the core right at the > start of a thread creation attempt. It tells the tool that the > specified ThreadId is now in use and that it can now expect to hear > of other events using that ThreadId. > > post_thread_create remains unchanged: as at present, it notifies the > tool that a thread has been successfully created. > > To make this completely general, perhaps we should also add > > threadid_inactive ( ThreadId ) > > which tells the tool that the given ThreadId is now inactive and > any further mention of it (except for threadid_active) is an error > in the core-tool protocol. > > ------- > > It further seems to me that this is complex enough that we should > use a state machine to specify the allowable sequences of thread > notification events in the core-tool interface. Roughly this would > be (note: this confuses 'events' with 'states') > > threadid_active -> post_thread_create // if successful > threadid_active -> threadid_inactive // if failed > > post_thread_create -> start_client_code // now running > > start_client_code -> stop_client_code // stopping > stop_client_code -> start_client_code // running again > > stop_client_code -> thread_join > > thread_join -> threadid_inactive > > > How does all that sound to you? > > J > The proposal sounds very nice. But I should have told you that I have an alternative available, namely allocating a new DrdThreadId each time a new ThreadId is passed by Valgrind to the drd tool. If the ThreadId of the creator thread is not yet available at the time the ThreadId of the created thread is passed for the first time to the drd tool, I will have to initialize the per-thread data in the drd tool in two steps anyway. Bart. |
|
From: Julian S. <js...@ac...> - 2006-12-29 19:24:14
|
> The proposal sounds very nice. But I should have told you that I have an > alternative available, namely allocating a new DrdThreadId each time a new > ThreadId is passed by Valgrind to the drd tool. If the ThreadId of the > creator thread is not yet available at the time the ThreadId of the created > thread is passed for the first time to the drd tool, I will have to > initialize the per-thread data in the drd tool in two steps anyway. That works I guess, but I would prefer to fix the core properly. I'll try to come up with a refined proposal for a state machine and post it. J |