----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3753/#review9197 -----------------------------------------------------------
src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 139) <http://reviews.gem5.org/r/3753/#comment7904> 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. src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 200) <http://reviews.gem5.org/r/3753/#comment7906> See my comment below in the function definition regarding the name of this variable ... src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 203) <http://reviews.gem5.org/r/3753/#comment7907> 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 } src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 264) <http://reviews.gem5.org/r/3753/#comment7905> 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. - Tushar Krishna 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 gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
