> On Oct. 17, 2016, 10:17 p.m., Michael LeBeane wrote:
> > src/sim/process.cc, line 447
> > <http://reviews.gem5.org/r/3676/diff/1/?file=61684#file61684line447>
> >
> >     Why remove?  Does this not work anymore?

The subscript accessor returns the fdentry directly so it has to discard the 
const qualifier for the (un)serialize methods.

Also, the checkpoints are broken so I'd like to revisit getting them working a 
single patch or a subset of patches. I don't think it's constructive to try to 
attack the checkpoint problem without having a clear goal on how to handle the 
outliers and things that make it general in difficult. For now, I'm ignoring 
the feature.

If you spend enough time looking at this patch and some of the subsequent ones 
(look at the fcntl patch and the flags field for an example), it becomes 
obvious that several of the fields in these classes are solely meant to hold 
the metadata needed for checkpoints. This seems contradictory given that I'm 
ignoring the feature. My goal right now is to make it apparent what types we're 
dealing with in the system call code, but also to make it possible to do 
checkpoints properly in the future by giving the metadata a structure to reside 
in.


- Brandon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3676/#review8881
-----------------------------------------------------------


On Oct. 17, 2016, 3:41 p.m., Brandon Potter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3676/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2016, 3:41 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11698:502f94aa9638
> ---------------------------
> syscall_emul: [patch 10/22] refactor fdentry and add fdarray class
> 
> Several large changes happen in this patch.
> 
> The FDEntry class is rewritten so that file descriptors now correspond to
> types: 'Regular' which is normal file-backed file with the file open on the
> host machine, 'Pipe' which is a pipe that has been opened on the host machine,
> and 'Device' which does not have an open file on the host yet acts as a pseudo
> device with which to issue ioctls. Other types which might be added in the
> future are directory entries and sockets (off the top of my head).
> 
> The FDArray class was create to hold most of the file descriptor handling
> that was stuffed into the Process class. It uses shared pointers and
> the std::array type to hold the FDEntries mentioned above. The implementation
> could use a review; I feel that there's some room for improvement, but it
> seems like a decent first step.
> 
> The changes to these two classes needed to be propagated out to the rest
> of the code so there were quite a few changes for that. Also, comments were
> added where I thought they were needed to help others and extend our
> DOxygen coverage.
> 
> 
> Diffs
> -----
> 
>   src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/fd_array.hh PRE-CREATION 
>   src/sim/fd_array.cc PRE-CREATION 
>   src/sim/fd_entry.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/fd_entry.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> 
> Diff: http://reviews.gem5.org/r/3676/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to