> On Sept. 1, 2012, 1:56 p.m., Nilay Vaish wrote: > > src/mem/ruby/system/RubyPort.cc, line 530 > > <http://reviews.gem5.org/r/1386/diff/2/?file=29383#file29383line530> > > > > Why has this function call changed to outstandingCount()?
First, since the RubyPort::drain should return the number of components that it is responsible for draining, I changed line 550 to include the RubyPort as a component that needs to be drained iff it has outstanding requests, instead of returning the number of outstanding requests that the RubyPort needs to drain. Previously, drainEvent->process() (line 534) should have been called on every ruby_hit_callback in order to clear all of the outstanding requests. However, it was only being called when there were no outstanding requests AND all of the child ports had drained (getDrainCount(drainEvent) = 0), so this resulted in a latent bug that if there was more than one outstanding request and/or undrained child port during drain, drainEvent->process() wouldn't even be called for each packet drained from the RubyPort or child port completed. This was very wrong. Second, the M5Port and PioPort (RubyPort's child ports) now descend from Queued*Ports, which use PacketQueues for buffering packets. These PacketQueues correctly obey the drain and process semantics that we clarified in the email thread yesterday. As such, the RubyPort child ports take responsibility for calling drainEvent->process() (in the PacketQueues) on their own when they've cleared their packets. Since the child ports are taken care of, the RubyPort is no longer responsible for checking up on the drain progress of its child ports, so it should definitely not be calling getDrainCount() here. This means that the RubyPort is only responsible for processing the drainEvent when it is completely drained (i.e. when it has no more requests outstanding => outstandingCount() = 0). - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1386/#review3380 ----------------------------------------------------------- On Aug. 31, 2012, 9:54 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, 9:54 p.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9185:725a5a991352 > --------------------------- > 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 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
