> On July 21, 2015, 8:50 p.m., Nilay Vaish wrote: > > src/sim/process.cc, line 316 > > <http://reviews.gem5.org/r/2966/diff/1/?file=48129#file48129line316> > > > > How does this change help? > > Brandon Potter wrote: > Rather than warn, I think it's better to generate an error (or ignore it > entirely). It's either a problem or it's not and I think having an invalid > value for the file descriptor is a problem. It might be better to be > strictly > rather than >= though, right?
Is fd ever < -1? If not, then this assert if of no use. I think we should have assert(fd > -1). > On July 21, 2015, 8:50 p.m., Nilay Vaish wrote: > > src/sim/process.hh, line 164 > > <http://reviews.gem5.org/r/2966/diff/1/?file=48128#file48128line164> > > > > I think this will break existing SE mode checkpoints. > > Brandon Potter wrote: > Is the problem that it's referenced through a pointer now? If not, can > you elaborate on why you think it might be a problem. (Sorry, don't have any > experience with using checkpoints in gem5 and only starting to need to > support the interface in code; we don't make heavy use of them internally.) I think we write all the fds whether they are valid or not to the checkpoint file. Since the number is changing from 256 to 1024, I think earlier checkpoints may not load. I think you should test with hello world application: take a checkpoint after 1000 instructions and see if the checkpoint loads correctly and the application runs to completion. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2966/#review6800 ----------------------------------------------------------- On July 14, 2015, 5:30 a.m., Brandon Potter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2966/ > ----------------------------------------------------------- > > (Updated July 14, 2015, 5:30 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10917:eafe9bd908a7 > --------------------------- > syscall_emul: file descriptor interface changes > > This patch gets rid of unused Process::dup_fd method and does minor > refactoring in the process class files. The file descriptor max has been > changed to be the number of file descriptors since this clarifies the loop > boundary condition and cleans up the code a bit. The fd_map field has been > altered to be dynamically allocated as opposed to being an array; the > intention here is to build on this is subsequent patches to allow processes > to share their file descriptors with the clone system call. > > > Diffs > ----- > > src/sim/process.hh 5c76426fd9ee08bca733582c8c2f001a99e7ff5a > src/sim/process.cc 5c76426fd9ee08bca733582c8c2f001a99e7ff5a > src/sim/syscall_emul.cc 5c76426fd9ee08bca733582c8c2f001a99e7ff5a > > Diff: http://reviews.gem5.org/r/2966/diff/ > > > Testing > ------- > > > Thanks, > > Brandon Potter > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
