I agree with your point about dup. I imagine we can keep an additional field
that determines whether a file descriptor was created by dup and if so,
re-open it with dup at the point of checkpoint restore. I will work on a fix
and post the diff.
Best,
-Rick
On Thu, Jan 20, 2011 at 10:32 PM, Steve Reinhardt <ste...@gmail.com> wrote:
> Hi Rick,
>
> Thanks for figuring this out. The last one is clearly just my dumb
> mistake; I just committed a fix. The serialization/unserialization of mode
> is an obvious enough oversight too.
>
> I'm not quite so sure about the dup fix though... I think the original
> sim_fd_obj() is doing the right thing, it's just that perhaps that's not
> what dupFunc() needs or wants. That just means dupFunc() shouldn't be
> calling it though. Maybe dupFunc() needs to call a different function that
> doesn't exist. However, I'm not so sure about that, because I'm not
> convinced the overall strategy for checkpointing and restoring dup'd fds is
> even right. At first glance, it looks like when we unserialize a pair of
> dup'd fds, we'll end up opening the file twice, rather than calling dup()
> again, which seems more correct.
>
> Steve
>
> On Thu, Jan 20, 2011 at 11:41 AM, Richard Strong <rstr...@cs.ucsd.edu>wrote:
>
>> Hi all,
>>
>> I uncovered 3 bugs when trying to resume in ALPHA_SE from a checkpoint for
>> the spec2006 benchmarks. They are:
>>
>> (1) Unserializing curTick failed because the checkpoint file wrote
>> curTick()=, but the unserializing code was expecting curTick=
>>
>> (2) The mode bit was not being stored in the file descriptor checkpoint. I
>> added a feature to store the mode bit.
>>
>> (3) The dup function was duplicating the wrong file descriptor.
>>
>> Let me know what you think, I give the diff below along with a traceflag
>> that allowed me to find the problem.
>>
>> Best,
>> -Rick
>>
>> diff --git a/src/sim/SConscript b/src/sim/SConscript
>> --- a/src/sim/SConscript
>> +++ b/src/sim/SConscript
>> @@ -77,3 +77,4 @@
>> TraceFlag('Thread')
>> TraceFlag('Timer')
>> TraceFlag('VtoPhys')
>> +TraceFlag('FileDescriptors')
>> diff --git a/src/sim/process.cc b/src/sim/process.cc
>> --- a/src/sim/process.cc
>> +++ b/src/sim/process.cc
>> @@ -268,6 +268,7 @@
>> int
>> Process::alloc_fd(int sim_fd, string filename, int flags, int mode, bool
>> pipe)
>> {
>> + DPRINTF(FileDescriptors, "alloc_fd fd:%d filename:%s\n", sim_fd,
>> filename);
>> // in case open() returns an error, don't allocate a new fd
>> if (sim_fd == -1)
>> return -1;
>> @@ -295,6 +296,7 @@
>> void
>> Process::free_fd(int tgt_fd)
>> {
>> + DPRINTF(FileDescriptors, "free_fd fd:%d name:%s\n", tgt_fd, "NULL");
>> Process::FdMap *fdo = &fd_map[tgt_fd];
>> if (fdo->fd == -1)
>> warn("Process::free_fd: request to free unused fd %d", tgt_fd);
>> @@ -324,8 +326,16 @@
>> {
>> if (tgt_fd > MAX_FD)
>> panic("sim_fd_obj called in fd out of range.");
>> +
>>
>> - return &fd_map[tgt_fd];
>> + //return &fd_map[tgt_fd];
>> + for (int x = 0; x <= MAX_FD; x++) {
>> + if (fd_map[x].fd == tgt_fd){
>> + return &fd_map[x];
>> + }
>> + }
>> + panic("sim_fd_obj could not find system fd.");
>> + return NULL;
>> }
>>
>> bool
>> @@ -458,6 +468,7 @@
>> if (fdo->fd != -1) {
>> fdo->fileOffset = lseek(fdo->fd, 0, SEEK_CUR);
>> } else {
>> + DPRINTF(FileDescriptors, "find_file_offsets fd:%d
>> filename:%s\n", fdo->fd, fdo->filename);
>> fdo->filename = "NULL";
>> fdo->fileOffset = 0;
>> }
>> @@ -480,6 +491,7 @@
>> SERIALIZE_SCALAR(flags);
>> SERIALIZE_SCALAR(readPipeSource);
>> SERIALIZE_SCALAR(fileOffset);
>> + SERIALIZE_SCALAR(mode);
>> }
>>
>> void
>> @@ -491,6 +503,7 @@
>> UNSERIALIZE_SCALAR(flags);
>> UNSERIALIZE_SCALAR(readPipeSource);
>> UNSERIALIZE_SCALAR(fileOffset);
>> + UNSERIALIZE_SCALAR(mode);
>> }
>>
>> void
>> diff --git a/src/sim/serialize.cc b/src/sim/serialize.cc
>> --- a/src/sim/serialize.cc
>> +++ b/src/sim/serialize.cc
>> @@ -411,7 +411,7 @@
>> {
>> const string §ion = name();
>> Tick tick;
>> - paramIn(cp, section, "curTick", tick);
>> + paramIn(cp, section, "curTick()", tick);
>> curTick(tick);
>>
>> mainEventQueue.unserialize(cp, "MainEventQueue");
>>
>>
>> _______________________________________________
>> m5-dev mailing list
>> m5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/m5-dev
>>
>>
>
> _______________________________________________
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
>
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
