> On Aug. 31, 2012, 7:06 p.m., Nilay Vaish wrote:
> > src/mem/ruby/system/Sequencer.cc, line 88
> > <http://reviews.gem5.org/r/1386/diff/1/?file=29382#file29382line88>
> >
> >     Are you sure this is correct?

wakeup() is only called when a deadlock event is processed. Since we don't want 
the deadlock event scheduled while draining, we can assert that wakeup() is 
never called while the RubyPort is in the draining state. This assertion was 
useful for sorting out the isDeadlockEventScheduled assertion failure in 
getDrainCount(), and it should be useful in the future if code changes.


> On Aug. 31, 2012, 7:06 p.m., Nilay Vaish wrote:
> > src/mem/ruby/system/Sequencer.cc, line 213
> > <http://reviews.gem5.org/r/1386/diff/1/?file=29382#file29382line213>
> >
> >     I think there is one more place in Sequencer.cc where the deadlock 
> > event is scheduled. You might want to put the same check there as well.

The above assert is that check.


> On Aug. 31, 2012, 7:06 p.m., Nilay Vaish wrote:
> > src/mem/ruby/system/RubyPort.cc, line 530
> > <http://reviews.gem5.org/r/1386/diff/1/?file=29381#file29381line530>
> >
> >     Why do you need to remove this local variable? Now you are calling 
> > outstandingCount() twice.

Will change this and resubmit.


- Joel


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


On Aug. 31, 2012, 4:56 p.m., Joel Hestness wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1386/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2012, 4:56 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Description
> -------
> 
> Changeset 9185:97d6b8c92b6d
> ---------------------------
> RubyPort and Sequencer: Fix draining
> 
> Fix the drain functionality of the RubyPort to only call drain on child ports
> during a system-wide drain process, instead of calling each time that a
> ruby_hit_callback is executed.
> 
> This fixes the issue of the RubyPort ports being reawakened during the drain
> simulation, possibly with work they didn't previously have to complete. If
> they have new work, they may call process on the drain event that they had
> not registered work for, causing an assertion failure in cleanupCountedDrain
> (assert(event->getCount() == 0);) when completing the drain event.
> 
> Also, in RubyPort, set the drainEvent to NULL when there are no events
> to be drained. If not set to NULL, the drain loop can result in stale
> drainEvents used.
> 
> 
> Diffs
> -----
> 
>   src/mem/ruby/system/RubyPort.cc 42807286d6cb 
>   src/mem/ruby/system/Sequencer.cc 42807286d6cb 
> 
> Diff: http://reviews.gem5.org/r/1386/diff/
> 
> 
> Testing
> -------
> 
> Verified that all prior working Ruby checkpointing configurations still work 
> correctly. Verified that this fixes checkpoint restore into timing CPU and 
> then switch to detailed CPU with x86. Will need to verify that this works 
> when restoring in full-system simulation, functionality which is currently 
> obscured by another bug when restoring into detailed CPU in FS.
> 
> 
> Thanks,
> 
> Joel Hestness
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to