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
