> On Dec. 9, 2016, 5:03 a.m., Tushar Krishna wrote: > > src/mem/ruby/network/garnet2.0/NetworkInterface.cc, line 277 > > <http://reviews.gem5.org/r/3753/diff/2/?file=63961#file63961line277> > > > > The name of this variable is a little confusing. > > > > > > Hmm I think I can see why you named it like that. If a message that was > > stalled gets unstalled, then this variable becomes true. > > > > But if the stall queue is empty, unstalledMessage is returned false, > > which means stalledMessage is true .. which is not quite right.
I see. How about messageEnqueued or similar to imply only one message per cycle is enqueued? > On Dec. 9, 2016, 5:03 a.m., Tushar Krishna wrote: > > src/mem/ruby/network/garnet2.0/NetworkInterface.cc, line 139 > > <http://reviews.gem5.org/r/3753/diff/2/?file=63961#file63961line139> > > > > If a flit is waiting for the output buffer to become free, this will > > get counted in its network delay component in the current implementation. > > It should be counted in its queueing delay component I think ... > > Queueing delay is the delay at the message buffers and NICs, network > > delay is from src NI to dest NI and includes delay in routers and links. That's true. I think this will need to be changed to account for queueing latency at the source plus time in stall queue. I could add a member/method to flit like get/set_dequeue_time which is set when the link is consumed. Then the calculation becomes `network_delay = t_flit->get_dequeue_time() - t_flit->get_enqueue_time()` and `queueing_delay = t_flit->get_src_delay() + (curCycle() - t_flit->get_dequeue_time())`. Will that suffice? We could also split the source and "destination" delays into two stats if we want. That would still require modifying flit class, though. > On Dec. 9, 2016, 5:03 a.m., Tushar Krishna wrote: > > src/mem/ruby/network/garnet2.0/NetworkInterface.cc, line 203 > > <http://reviews.gem5.org/r/3753/diff/2/?file=63961#file63961line203> > > > > We should consume the link irrespective of whether the state of the > > stall queue. If we do not consume, effectively we are modeling a quueue > > inside the network link. > > > > I can see why you added this condition though - you don't want more > > than one enqueue into the message buffer in the same cycle. > > If the stall queue already enqueued something, you are making the one > > from the link wait. > > > > How about the following: > > > > bool unstalledMessage = checkStallQueue(); > > > > if (inNetLink->isReady(curCycle()) { > > flit *t_flit = inNetLink->consumeLink(); > > > > ... > > if (outNode_ptr[vnet]->areNSlotsAvailable(1, curTime) && > > !unstalledMessage) { > > // enqueue into protocol buffer > > } > > else { > > // enqueue into stall queue > > } Ok, I understand this now. The networkLink is internally modeled as a std::vector and becomes a queue. I will make the suggested change. - Matthew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3753/#review9197 ----------------------------------------------------------- On Dec. 9, 2016, 1:58 a.m., Matthew Poremba wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3753/ > ----------------------------------------------------------- > > (Updated Dec. 9, 2016, 1:58 a.m.) > > > Review request for Default and Tushar Krishna. > > > Repository: gem5 > > > Description > ------- > > Changeset 11761:fd4126ef5e73 > --------------------------- > ruby: Check MessageBuffer space in garnet NetworkInterface > > Garnet's NetworkInterface does not consider the size of MessageBuffers when > ejecting a Message from the network. Add a size check for the MessageBuffer > and only enqueue if space is available. If space is not available, the > message if placed in a queue and the credit is held. A callback from the > MessageBuffer is implemented to wake the NetworkInterface. If there are > messages in the stalled queue, they are processed first, in a FIFO manner > and if succesfully ejected, the credit is finally sent back upstream. The > maximum size of the stall queue is equal to the number of valid VNETs > with MessageBuffers attached. > > > Diffs > ----- > > src/mem/ruby/network/MessageBuffer.hh > 78ef8daecd81de0c392034809b3bc155396bf983 > src/mem/ruby/network/MessageBuffer.cc > 78ef8daecd81de0c392034809b3bc155396bf983 > src/mem/ruby/network/garnet2.0/NetworkInterface.hh > 78ef8daecd81de0c392034809b3bc155396bf983 > src/mem/ruby/network/garnet2.0/NetworkInterface.cc > 78ef8daecd81de0c392034809b3bc155396bf983 > > Diff: http://reviews.gem5.org/r/3753/diff/ > > > Testing > ------- > > > Thanks, > > Matthew Poremba > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
