> On Oct. 19, 2016, 5:50 p.m., Steve Reinhardt wrote:
> > 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.

I implemented Steve's recommendations. The implementation went into the 
clone/exec patch which I will repost with the update.

There is a default value for PID set to 1000. The configs allow a PID to be 
specified and there is a check inside the Process constructor which accesses 
the System object and into a PID set to check if the PID is already used; the 
constructor issues a fatal is there is a PID collision. Within the clone system 
call, there is a mechanism that incrementally checks through the PID set and 
increments through it until it finds a free PID.


- Brandon


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


On Oct. 17, 2016, 3:14 p.m., Brandon Potter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3670/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2016, 3:14 p.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

Reply via email to