syscall_emul: [patch 4/22] remove redundant M5_pid field from process
Review Request #3670 - Created Oct. 17, 2016 and submitted
Information | |
---|---|
Brandon Potter | |
gem5 | |
default | |
Reviewers | |
Default | |
Changeset 11692:ba60e4ae816f --------------------------- syscall_emul: [patch 4/22] remove redundant M5_pid field from process
With this patch, if you run multiple processes in SE mode they all have the same PID (100) by default. Will this cause issues? I haven't tested a pthreads application, but that seems like it might cause problems.
Would you just suggest that we make sure to assign different PIDs in the Python config file (where we create the Process object) to get around this issue? If so, we should have a fatal if there are multiple Processes with the same PID.
Summary: |
|
|||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||||||||
Diff: |
Revision 2 (+35 -50) |
-
src/sim/process.cc (Diff revision 2) -
Even if we are serializing nextPid in the System object, shouldn't we still serialize _pid? We might need to have the same pids restored and not reassign different ones.
-
src/sim/process.cc (Diff revision 2) -
Same goes for unserialization.
I agree that it's dumb to have two PIDs in the Process object, but until we figure out how we really want to fix the auto-assignment thing, it's probably premature to push this patch IMO. Let's wait until we have a full solution.
I'd be fine with doing something in python rather than C++; the way I see it, we already assign default values in python, so adding code to assign unique PIDs is really just a "smart default". That said, strictly speaking the Process class is not the best place for this, since really you'd want separate next-PID counters per System object. Making that happen automagically would be tricky though, and even the tricky solutions I can think of would be fragile in terms of perhaps swithcing around PID assignments based on unrelated config script changes. So off the top of my head, here's a proposal:
- We continue to provide a fixed default PID in python to keep things simple for single-process runs.
- For configs where the user is creating multiple Process objects per System, it's up to the user to manually assign unique PIDs. I think this is a good solution because it's an easy thing for users to do (just implement their own local counter in the python script), and yet is trivial from our side too. If we really wanted to, we could provide a get_next_pid() method on the Python System object, but I think that's overkill.
- The System object needs to track all the PIDs of all the Process objects that are assigned to it at config time, and (1) call fatal() if the user builds a config with non-unique PIDs (with a helpful error message of course), and (2) do something to figure out how it can continue to assign unique PIDs when clone() is called. It might be possible to do #1 in python, but it's probably easier and more robust to do it in C++. Part 2 definitely needs to be on the C++ side, and could be as complicated as keeping a map of which PIDs are used, or as simple as just tracking the max manually assigned PID and then incrementing from there.
Ship It!
Ship It!
Ship It!