> On Sept. 22, 2015, 2:03 p.m., Andreas Hansson wrote: > > src/mem/cache/cache.cc, line 1771 > > <http://reviews.gem5.org/r/3047/diff/1/?file=49087#file49087line1771> > > > > The trick here is in the difference between snoopDelay and headerDelay. > > > > Snoop delay is where we do the max on each level. Header delay is just > > used to eventually pay for it and pass it "downwards". > > Nilay Vaish wrote: > Here is what I think you are doing. Suppose we have three levels of > caching. We have three private > controllers (A1,B1,C1) at the first level, another three private > controllers at the second level > (A2,B2,C2) and one single shared controller (S) at the third level. When > S receives a packet from > any of the second level controllers, it forwards those packets to the > other second level > controllers so that they can invalidate or writeback blocks if they need > to. > > Now suppose A2 sent a packet to S which S forwarded to B2 and C2. Then, > B2 and C2 would update the snoop delay > of packet with their lookup latency and the header delay with the total > delay of snoops sent to B1 and C1. > Since all this was happening in parallel (which is why I think you take > max), should not the header delay > be also maximum of the delays of snoops sent to B1 and C1. > > I might be completely incorrect here.
Who needs Sudokus... :-). I think you're right in that I've messed this up. The packet from A2 will be forwarded to B2 and C2 by the crossbar in front of S. First we forward it to B2, and B2 updates the snoopDelay with a max expression, and then takes a copy (snoopPkt), then forwards that to B1. On returning from the upwards snoop, the headerDelay from the copied snoopPkt is added to the original packets headerDelay (and not snoopDelay). I think this is indeed wrong, we should add it to the snoopDelay rather, and preferably do so before the max expression. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3047/#review7243 ----------------------------------------------------------- On Aug. 19, 2015, 9:06 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3047/ > ----------------------------------------------------------- > > (Updated Aug. 19, 2015, 9:06 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11058:55f9e34a82d7 > --------------------------- > mem: Make the coherent crossbar account for timing snoops > > This patch introduces the concept of a snoop latency. Given the > requirement to snoop and forward packets in zero time (due to the > coherency mechanism), the latency is accounted for later. > > On a snoop, we establish the latency, and later add it to the header > delay of the packet. To allow multiple caches to contribute to the > snoop latency, we use a separate variable in the packet, and then take > the maximum before adding it to the header delay. > > > Diffs > ----- > > src/mem/cache/cache.cc 110cce93d398 > src/mem/coherent_xbar.cc 110cce93d398 > src/mem/packet.hh 110cce93d398 > > Diff: http://reviews.gem5.org/r/3047/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
