----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3670/#review8930 -----------------------------------------------------------
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: 1. We continue to provide a fixed default PID in python to keep things simple for single-process runs. 2. 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. 3. 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. - Steve Reinhardt On Oct. 17, 2016, 8:14 a.m., Brandon Potter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3670/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2016, 8:14 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11692:ba60e4ae816f > --------------------------- > syscall_emul: [patch 4/22] remove redundant M5_pid field from process > > > Diffs > ----- > > src/sim/system.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/system.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/Process.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/faults.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3670/diff/ > > > Testing > ------- > > > Thanks, > > Brandon Potter > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
