|
From: Nicholas N. <nj...@ca...> - 2004-07-26 15:55:06
Attachments:
diff
|
Jeremy, I think the as_pad() call in vg_main.c:layout_client_space() is redundant. It pads the area VG_(client_base)..VG_(valgrind_base) (which is 0..0xb0000000). But in stage1.c:hoops(), just before stage2 is launched, there is another as_pad() which does the area 0..info.map_base (0..0xb1000000). So I think the second one is redundant, both with the current values of the relevant variables, and any future ones (since VG_(client_base) will always be >= 0, and VG_(valgrind_base) will always be < info.mapbase. The attached diff removes the redundant as_pad() in vg_main.c, which allows layout_client_space() to merge with layout_remaining_space. It works fine on my machine. Does it seem ok to you? I'll commit if so. N |
|
From: Jeremy F. <je...@go...> - 2004-07-26 19:55:43
|
On Mon, 2004-07-26 at 16:54 +0100, Nicholas Nethercote wrote: > Jeremy, > > I think the as_pad() call in vg_main.c:layout_client_space() is redundant. > It pads the area VG_(client_base)..VG_(valgrind_base) (which is > 0..0xb0000000). > > But in stage1.c:hoops(), just before stage2 is launched, there is another > as_pad() which does the area 0..info.map_base (0..0xb1000000). > > So I think the second one is redundant, both with the current values of > the relevant variables, and any future ones (since VG_(client_base) will > always be >= 0, and VG_(valgrind_base) will always be < info.mapbase. > > The attached diff removes the redundant as_pad() in vg_main.c, which > allows layout_client_space() to merge with layout_remaining_space. It > works fine on my machine. Does it seem ok to you? I'll commit if so. That seems OK, but I think I put the second as_pad there deliberately, even though its redundant. Mainly, I think, to reduce the number of implicit dependencies between stage1 and stage2. The as_pad in stage1 is functionally there to make sure that stage2 loads in the right place; the one in stage2 is to make sure the tools load in the right place. Hm, it isn't a very strong reason for keeping it there. J |
|
From: Nicholas N. <nj...@ca...> - 2004-07-28 08:36:08
|
On Mon, 26 Jul 2004, Jeremy Fitzhardinge wrote: >> The attached diff removes the redundant as_pad() in vg_main.c, which >> allows layout_client_space() to merge with layout_remaining_space. It >> works fine on my machine. Does it seem ok to you? I'll commit if so. > > That seems OK, but I think I put the second as_pad there deliberately, > even though its redundant. Mainly, I think, to reduce the number of > implicit dependencies between stage1 and stage2. The as_pad in stage1 > is functionally there to make sure that stage2 loads in the right place; > the one in stage2 is to make sure the tools load in the right place. > > Hm, it isn't a very strong reason for keeping it there. Especially since stage2 aborts if stage1 didn't launch it. And there doesn't seem to be any use in allowing the possibility of stage2 being launched on its own? N |
|
From: Nicholas N. <nj...@ca...> - 2004-07-28 10:02:41
Attachments:
diff
|
On Wed, 28 Jul 2004, Nicholas Nethercote wrote: >> That seems OK, but I think I put the second as_pad there deliberately, >> even though its redundant. Mainly, I think, to reduce the number of >> implicit dependencies between stage1 and stage2. The as_pad in stage1 >> is functionally there to make sure that stage2 loads in the right place; >> the one in stage2 is to make sure the tools load in the right place. >> >> Hm, it isn't a very strong reason for keeping it there. > > Especially since stage2 aborts if stage1 didn't launch it. And there doesn't > seem to be any use in allowing the possibility of stage2 being launched on > its own? Here's a more compelling patch: I've rejigged as_*() so that no implicit state is kept in global variables in ume.c; the padfile fd is now threaded through explicitly. The as_*() interface has changed from this void as_pad(void *start, void *end); void as_unpad(void *start, void *end); void as_closepadfile(void); int as_getpadfd(void); void as_setpadfd(int); to this int as_openpadfile (void); void as_pad (void *start, void *end, int padfile); void as_unpad (void *start, void *end, int padfile); void as_closepadfile(int padfile); which is a definite improvement. Jeremy, can you check this? If there are no complaints I'll commit. ---- While I'm at it, I noticed the function declarations in ume.h do not have "extern" -- does this matter? N |
|
From: Tom H. <th...@cy...> - 2004-07-28 10:14:51
|
In message <Pin...@he...>
Nicholas Nethercote <nj...@ca...> wrote:
> While I'm at it, I noticed the function declarations in ume.h do not
> have "extern" -- does this matter?
No - extern is the default for function declarations in C.
Tom
--
Tom Hughes (th...@cy...)
Software Engineer, Cyberscience Corporation
http://www.cyberscience.com/
|
|
From: Nicholas N. <nj...@ca...> - 2004-07-30 11:44:26
|
On Wed, 28 Jul 2004, Nicholas Nethercote wrote: > Here's a more compelling patch: I've rejigged as_*() so that no implicit > state is kept in global variables in ume.c; the padfile fd is now threaded > through explicitly. The as_*() interface has changed from this > > void as_pad(void *start, void *end); > void as_unpad(void *start, void *end); > void as_closepadfile(void); > int as_getpadfd(void); > void as_setpadfd(int); > > to this > > int as_openpadfile (void); > void as_pad (void *start, void *end, int padfile); > void as_unpad (void *start, void *end, int padfile); > void as_closepadfile(int padfile); > > which is a definite improvement. Jeremy, can you check this? If there are > no complaints I'll commit. Ok, I will commit this later today unless there are any last-minute objections. N |
|
From: Jeremy F. <je...@go...> - 2004-07-30 19:17:06
|
On Fri, 2004-07-30 at 12:44 +0100, Nicholas Nethercote wrote: > Ok, I will commit this later today unless there are any last-minute > objections. Good. J |