-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/68/#review115
-----------------------------------------------------------


Thanks!  Just a couple minor things...

- Did you test this with the other CPU models besides O3?  I'm guessing you did 
and O3 was the only one that broke, but I just wanted to verify that.



src/arch/alpha/linux/process.cc
<http://reviews.m5sim.org/r/68/#comment254>

    This is a broader question (not just for Ioannis): does it make sense to 
proactively add this to the syscall tables for the other ISAs too?  I don't see 
anything ISA-specific here.



src/cpu/o3/cpu.cc
<http://reviews.m5sim.org/r/68/#comment252>

    I'm curious why the state is set to idle even if there is one active 
thread... I see that the existing test above in deallocateContext is similar 
but I don't understand that one either :-).  Adding a comment to cover this 
would be helpful.



src/sim/syscall_emul.hh
<http://reviews.m5sim.org/r/68/#comment253>

    Some of the lines in this function look >80 chars; if so they should be 
wrapped.  (Anyone know if there's a way to check this in ReviewBoard?)


- Steve


On 2010-07-29 07:59:26, Ioannis Ilkos wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/68/
> -----------------------------------------------------------
> 
> (Updated 2010-07-29 07:59:26)
> 
> 
> Review request for Default.
> 
> 
> Summary
> -------
> 
> This is a patch fixing the ThreadContext suspension / activation described in 
> http://www.mail-archive.com/m5-dev@m5sim.org/msg07307.html and implementing 
> the nanosleep() syscall (albeit without the signal-relevant parts) for 
> AlphaLinux.
> 
> Changes in O3:
> - The tick scheduling was removed from activateContext() and moved into 
> activateThread(). It seems more natural since activateContext() either calls 
> activateThread() or schedules it. In the case of scheduling there is no need 
> to schedule ticks prematurely.
> - suspendContext() and haltContext() check the number of active threads 
> before setting CPU _status to Idle.
> 
> 
> Diffs
> -----
> 
>   src/arch/alpha/linux/linux.hh b28e7286990c 
>   src/arch/alpha/linux/process.cc b28e7286990c 
>   src/cpu/o3/cpu.cc b28e7286990c 
>   src/sim/syscall_emul.hh b28e7286990c 
> 
> Diff: http://reviews.m5sim.org/r/68/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ioannis
> 
>

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

Reply via email to